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

[Andrew Morton]

On Wed, 7 Aug 2013 17:12:36 -0400 Ed Cashin <ecashin@xxxxxxxxxx> wrote:

> 
> On Aug 7, 2013, at 4:58 PM, Andrew Morton wrote:
> 
> > On Thu, 1 Aug 2013 21:29:59 -0400 Ed Cashin <ecashin@xxxxxxxxxx> wrote:
> > 
> >> As discussed previously,
> > 
> > I think I missed that.
> > 
> >> the fact that some users of the block
> >> layer provide bios that point to pages with a zero _count means
> >> that it is not OK for the network layer to do a put_page on the
> >> skb frags during an skb_linearize, so the aoe driver gets a
> >> reference to pages in bios and puts the reference before ending
> >> the bio.  And because it cannot use get_page on a page with a
> >> zero _count, it manipulates the value directly.
> > 
> > Eh?  What code is putting count==0 pages into bios?  That sounds very
> > weird and broken.
> 
> I thought so in 2007 but couldn't solicit a clear "this is wrong" consensus from the discussion.
> 
>   http://article.gmane.org/gmane.linux.kernel/499197
>   https://lkml.org/lkml/2007/1/19/56
>   https://lkml.org/lkml/2006/12/18/230
> 
> We were seeing zero-count pages in bios from XFS, but Christoph Hellwig pointed out that kmalloced pages can also come from ext3 when it's doing log recovery, and they'll have zero page counts.

aiiee!

It is (I suppose) reasonable to put kmalloced memory into a BIO's page
array.  And it is perfectly reasonable for a user of that bio to do a
get_page/put_page against that page.  It is utterly unreasonable for
the damn page to get freed as a result!

I'd claim that slab is broken.  The page is in use, so it should have an
elevated refcount, full stop.

--
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/