Re: [PATCH] aoe: adjust ref of head for compound page tails

[Andrew Morton]

On Wed, 7 Aug 2013 19:27:40 -0400 Ed Cashin <ecashin@xxxxxxxxxx> wrote:

> On Aug 7, 2013, at 5:26 PM, Andrew Morton wrote:
> 
> > On Wed, 7 Aug 2013 17:21:32 -0400 Ed Cashin <ecashin@xxxxxxxxxx> wrote:
> ...
> >> Sorry.  I tried to cover that but maybe should have separated it more clearly from the running text.
> >> 
> >> The patch changes the current behavior encountered with direct I/O and aoe:
> >> 
> >>  aoe can BUG when direct I/O used
> >> 
> >> ... to a new behavior:
> >> 
> >>  aoe does not BUG when direct I/O is used
> >> 
> >> And the user will never know that instead aoe is manipulating the _count of the head page, even though that's what's happening with this patch applied.
> >> 
> >> (The user-visible effects that motivated the older patch that added the BUG to aoe when page tails are used for block I/O was a VM_BUG_ON that occurred when the aoe driver manipulated the _count of a compound page tail.)
> > 
> > OK, I added "Fix a BUG which can trigger when direct-IO is used with
> > AOE".  Which kernel versions are affected?
> 
> 
> That BUG was added to the aoe driver in a commit that first appears in v3.7.  It persists until this patch adds the code that handles the situation that the BUG was intended to detect.
> 
> [ecashin@marino linux]$ git describe --contains --match 'v*' 69cf2d85de773d998798e47e3335b85e5645d157
> v3.7-rc1~110^2~22
> [ecashin@marino linux]$ 
> 

ah, is it this?

+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.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/