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 <mho...@suse.com> Thanks! -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
