Re: Suspend2 Merge: e820 table support.

[Pavel Machek]

Hi!

> > Hmm, it also contains (saveable()):
> > 
> >         BUG_ON(PageReserved(page) && PageNosave(page));
> 
> How do you cover those HighMem pages that get marked Reserved and are
> unusable? (That's what the e820 logic was for, iirc. Think it was done
> about February!). Not handling them resulted in MCEs when trying to do
> the atomic copy or when restoring (seemed random).

This function is not use for highmem, AFAICS. If page is marked
reserved we do not touch it. Do you suggest that we need to save it
for highmem case?

MCEs... I see you have patch to disable them during suspend... That's
clearly wrong thing to do, right?

> > ..but that should be easy to kill. I'd be worried about this function:
> > 
> > static void free_suspend_pagedir_zone(struct zone *zone, unsigned long
> > pagedir)
> > {
> >         unsigned long zone_pfn, pagedir_end, pagedir_pfn,
> > pagedir_end_pfn;
> >         pagedir_end = pagedir + (PAGE_SIZE << pagedir_order);
> >         pagedir_pfn = __pa(pagedir) >> PAGE_SHIFT;
> >         pagedir_end_pfn = __pa(pagedir_end) >> PAGE_SHIFT;
> >         for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
> > {
> >                 struct page *page;
> >                 unsigned long pfn = zone_pfn + zone->zone_start_pfn;
> >                 if (!pfn_valid(pfn))
> >                         continue;
> >                 page = pfn_to_page(pfn);
> 
> Mmm. I should get around to using pfn_to_page. That's necessary for
> discontig support, right? Haven't looked at that yet. (Yes, swsusp has
> functionality suspend2 doesn't!) :>.
> 
> >                 if (!TestClearPageNosave(page))
> >                         continue;
> >                 else if (pfn >= pagedir_pfn && pfn < pagedir_end_pfn)
> >                         continue;
> >                 __free_page(page);
> >         }
> > }
> 
> Wow. This function is really hard to understand. Or maybe I'm really
> ignorant :>.

No, this function really is ugly and needs to die.

								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/