On Fri 03-09-10 18:13:27, KAMEZAWA Hiroyuki wrote:
> On Fri, 3 Sep 2010 10:25:58 +0200
> Michal Hocko <mhocko@xxxxxxx> wrote:
>
> > On Fri 03-09-10 12:14:52, KAMEZAWA Hiroyuki wrote:
> > [...]
[...]
> > Cannot ZONE_MOVABLE contain different MIGRATE_types?
> >
> never.
Then I am terribly missing something. Zone contains free lists for
different MIGRATE_TYPES, doesn't it? Pages allocated from those free
lists keep the migration type of the list, right?
ZONE_MOVABLE just says whether it makes sense to move pages in that zone
at all, right?
>
> > > +
> > > + pfn = page_to_pfn(page);
> > > + for (found = 0, iter = 0; iter < pageblock_nr_pages; iter++) {
> > > + unsigned long check = pfn + iter;
> > > +
> > > + if (!pfn_valid_within(check)) {
> > > + iter++;
> > > + continue;
> > > + }
> > > + page = pfn_to_page(check);
> > > + if (!page_count(page)) {
> > > + if (PageBuddy(page))
> >
> > Why do you check page_count as well? PageBuddy has alwyas count==0,
> > right?
> >
>
> But PageBuddy() flag is considered to be valid only when page_count()==0.
> This is for safe handling.
OK. I don't see that documented anywhere but it makes sense. Anyway
there are some places which don't do this test (e.g.
isolate_freepages_block, suitable_migration_target, etc.).
>
>
> > > + iter += (1 << page_order(page)) - 1;
> > > + continue;
> > > + }
> > > + if (!PageLRU(page))
> > > + found++;
> > > + /*
> > > + * If the page is not RAM, page_count()should be 0.
> > > + * we don't need more check. This is an _used_ not-movable page.
> > > + *
> > > + * The problematic thing here is PG_reserved pages. But if
> > > + * a PG_reserved page is _used_ (at boot), page_count > 1.
> > > + * But...is there PG_reserved && page_count(page)==0 page ?
> >
> > Can we have PG_reserved && PG_lru?
>
> I think never.
>
> > I also quite don't understand the comment.
>
> There an issue that "remove an memory section which includes memory hole".
> Then,
>
> a page used by bootmem .... PG_reserved.
> a page of memory hole .... PG_reserved.
>
> We need to call page_is_ram() or some for handling this mess.
OK, I see.
>
>
> > At this place we are sure that the page is valid and neither
> > free nor LRU.
> >
[...]
> > > +bool is_pageblock_removable(struct page *page)
> > > +{
> > > + struct zone *zone = page_zone(page);
> > > + unsigned long flags;
> > > + int num;
> > > +
> > > + spin_lock_irqsave(&zone->lock, flags);
> > > + num = __count_unmovable_pages(zone, page);
> > > + spin_unlock_irqrestore(&zone->lock, flags);
> >
> > Isn't this a problem? The function is triggered from userspace by sysfs
> > (0444 file) and holds the lock for pageblock_nr_pages. So someone can
> > simply read the file and block the zone->lock preventing/delaying
> > allocations for the rest of the system.
> >
> But we need to take this. Maybe no panic you'll see even if no-lock.
Yes, I think that this can only lead to a false possitive in sysfs
interface. Isolating code holds the lock.
Thanks
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>