Re: [PATCHv2 4/4] mm: make compound_head() robust
From: Kirill A. Shutemov
Date: Tue Aug 18 2015 - 14:24:50 EST
On Tue, Aug 18, 2015 at 06:41:13PM +0200, Michal Hocko wrote:
> On Tue 18-08-15 13:20:22, Michal Hocko wrote:
> > On Mon 17-08-15 18:09:05, Kirill A. Shutemov wrote:
> > > Hugh has pointed that compound_head() call can be unsafe in some
> > > context. There's one example:
> > >
> > > CPU0 CPU1
> > >
> > > isolate_migratepages_block()
> > > page_count()
> > > compound_head()
> > > !!PageTail() == true
> > > put_page()
> > > tail->first_page = NULL
> > > head = tail->first_page
> > > alloc_pages(__GFP_COMP)
> > > prep_compound_page()
> > > tail->first_page = head
> > > __SetPageTail(p);
> > > !!PageTail() == true
> > > <head == NULL dereferencing>
> > >
> > > The race is pure theoretical. I don't it's possible to trigger it in
> > > practice. But who knows.
> > >
> > > We can fix the race by changing how encode PageTail() and compound_head()
> > > within struct page to be able to update them in one shot.
> > >
> > > The patch introduces page->compound_head into third double word block in
> > > front of compound_dtor and compound_order. That means it shares storage
> > > space with:
> > >
> > > - page->lru.next;
> > > - page->next;
> > > - page->rcu_head.next;
> > > - page->pmd_huge_pte;
> > >
> > > That's too long list to be absolutely sure, but looks like nobody uses
> > > bit 0 of the word. It can be used to encode PageTail(). And if the bit
> > > set, rest of the word is pointer to head page.
> >
> > I didn't look too closely but the general idea makes sense to me and the
> > overal code simplification is sound. I will give it more detailed review
> > after I sort out other stuff.
>
> AFICS page::first_page wasn't used outside of compound page logic so you
> should remove it in this patch. The rest looks good to me.
I missed it by accident during rework for v2. Will fix.
> Acked-by: Michal Hocko <mhocko@xxxxxxxx>
Thanks!
--
Kirill A. Shutemov
--
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/