lkml.org 
[lkml]   [2010]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] Make is_mem_section_removable more conformable with offlining code
    On Fri 03-09-10 18:13:27, KAMEZAWA Hiroyuki wrote:
    > On Fri, 3 Sep 2010 10:25:58 +0200
    > Michal Hocko <mhocko@suse.cz> 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


    \
     
     \ /
      Last update: 2010-09-03 11:53    [W:0.023 / U:2.376 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site