Messages in this thread | | | Date | Thu, 8 Aug 2013 12:56:29 -0700 | From | Andrew Morton <> | Subject | Re: [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.
| |