lkml.org 
[lkml]   [2013]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] aoe: adjust ref of head for compound page tails
On Wed, 7 Aug 2013 20:50:09 -0400 Ed Cashin <ecashin@coraid.com> wrote:

> > But we shouldn't get that far:
> >
> > static inline void get_page(struct page *page)
> > {
> > if (unlikely(PageTail(page)))
> > if (likely(__get_page_tail(page)))
> > return;
> > /*
> > * Getting a normal page or the head of a compound page
> > * requires to already have an elevated page->_count.
> > */
> > VM_BUG_ON(atomic_read(&page->_count) <= 0);
> > atomic_inc(&page->_count);
> > }
> >
> > This is a tail page, so we should be using __get_page_tail().
>
>
> When the workaround was created, it was with the assumption that the zero-count pages are not always tail pages, and that seemed to be the case in 2007, but as I said, I don't have a mechanism for detecting that now, so I cannot say whether it really happens with today's kernel.

It sounds we should pull out all that code and retest. It shouldn't be
needed - if this results in some failure then I suspect core MM will
need changes.

Why don't you have a "mechanism for detecting that"? It's a matter of
pointing AOE at some hugetlb pages?

> If it is never correct for normal pages or compound page heads to have a zero count while they are associated with a bio, then yes, I think get_page is a great solution. The VM_BUG_ON in get_page would identify any parts of the kernel that are supplying bios that have pages without references.
>
> Just a note in response to "we shouldn't get that far": I believe the VM_BUG_ON line in get_page does get executed for tail pages when the __get_page_tail returns false because the compound page head has a zero count:
>
> get_page -> __get_page_tail -> get_page_unless_zero returns false, so
> __get_page_tail returns "got", which is still false, so
> get_page executes the VM_BUG_ON, where the _count will be zero for the tail page.

Yup. If we hit a zero-ref huge page here then we want that BUG_ON to trigger.



\
 
 \ /
  Last update: 2013-08-08 22:21    [W:0.111 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site