Messages in this thread | | | Date | Fri, 3 Jun 2011 05:21:56 +0900 | From | Minchan Kim <> | Subject | Re: [PATCH] mm: compaction: Abort compaction if too many pages are isolated and caller is asynchronous |
| |
On Thu, Jun 02, 2011 at 08:23:02PM +0200, Andrea Arcangeli wrote: > On Tue, May 31, 2011 at 11:38:30PM +0900, Minchan Kim wrote: > > > Yes. You find a new BUG. > > > It seems to be related to this problem but it should be solved although > > > > typo : It doesn't seem to be. > > This should fix it, but I doubt it matters for this problem. > > === > Subject: mm: no page_count without a page pin > > From: Andrea Arcangeli <aarcange@redhat.com> > > It's unsafe to run page_count during the physical pfn scan. > > Signed-off-by: Andrea Arcangeli <aarcange@redhat.com> > --- > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index faa0a08..e41e78a 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1124,8 +1124,18 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, > nr_lumpy_dirty++; > scan++; > } else { > - /* the page is freed already. */ > - if (!page_count(cursor_page)) > + /* > + * We can't use page_count() as that > + * requires compound_head and we don't > + * have a pin on the page here. If a > + * page is tail, we may or may not > + * have isolated the head, so assume > + * it's not free, it'd be tricky to
Isn't it rather aggressive? I think cursor page is likely to be PageTail rather than PageHead. Could we handle it simply with below code?
get_page(cursor_page) /* The page is freed already */ if (1 == page_count(cursor_page)) { put_page(cursor_page) continue; } put_page(cursor_page);
> + * track the head status without a > + * page pin. > + */ > + if (!PageTail(cursor_page) && > + !atomic_read(&cursor_page->_count)) > continue; > break;
> }
-- Kind regards Minchan Kim
| |