Messages in this thread | | | Date | Wed, 7 Aug 2013 17:05:48 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] aoe: adjust ref of head for compound page tails |
| |
On Wed, 7 Aug 2013 19:54:45 -0400 Ed Cashin <ecashin@coraid.com> wrote:
> > +bio_pageinc(struct bio *bio) > > +{ > > + struct bio_vec *bv; > > + struct page *page; > > + int i; > > + > > + bio_for_each_segment(bv, bio, i) { > > + page = bv->bv_page; > > + /* Non-zero page count for non-head members of > > + * compound pages is no longer allowed by the kernel, > > + * but this has never been seen here. > > + */ > > + if (unlikely(PageCompound(page))) > > + if (compound_trans_head(page) != page) { > > + pr_crit("page tail used for block I/O\n"); > > + BUG(); > > + } > > > > But get_page/put_page against a tail page of a compound page should > > Just Work. The core MM will hunt down the head page and manipulate its > > refcount. > > > > Perhaps the problem here is that AOE is diving under the covers and is > > using low-level page refcount alterations: > > > > + atomic_inc(&page->_count); > > > > Why does AOE do this? It would be better if it were to use the > > official published MM interfaces. If those interfaces need > > alteration/augmentation then we can do that. > > The get_page cannot be used when the _count is 0 (which is exactly when we need to increment it), because it will VM_BUG_ON. > > /* > * 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); >
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().
| |