From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755275Ab3LSTJD (ORCPT ); Thu, 19 Dec 2013 14:09:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:26604 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755179Ab3LSTJB (ORCPT ); Thu, 19 Dec 2013 14:09:01 -0500 Date: Thu, 19 Dec 2013 20:09:20 +0100 From: Oleg Nesterov To: Andrea Arcangeli Cc: Thomas Gleixner , Linus Torvalds , Dave Jones , Darren Hart , Linux Kernel Mailing List , Peter Zijlstra , Mel Gorman , Martin Schwidefsky , Heiko Carstens Subject: [PATCH 1/1] mm: fix the theoretical compound_lock() vs prep_new_page() race Message-ID: <20131219190920.GB24566@redhat.com> References: <20131211170844.GA21700@redhat.com> <20131211175615.GA24546@redhat.com> <20131211191855.GA32485@redhat.com> <20131213151035.GE5408@redhat.com> <20131213162240.GA11762@redhat.com> <20131213173406.GG5408@redhat.com> <20131216183618.GA28252@redhat.com> <20131216201952.GE21218@redhat.com> <20131219190846.GA24566@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20131219190846.GA24566@redhat.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org get/put_page(thp_tail) paths do get_page_unless_zero(page_head) + compound_lock(). In theory this page_head can be already freed and reallocated as alloc_pages(__GFP_COMP, smaller_order). In this case get_page_unless_zero() can succeed right after set_page_refcounted(), and compound_lock() can race with the non-atomic __SetPageHead(). Perhaps we should rework the thp locking (under discussion), but until then this patch moves set_page_refcounted() and adds wmb() to ensure that page->_count != 0 comes as a last change. I am not sure about other callers of set_page_refcounted(), but at first glance they look fine to me. Signed-off-by: Oleg Nesterov --- mm/page_alloc.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 115b23b..9402337 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -865,8 +865,6 @@ static int prep_new_page(struct page *page, int order, gfp_t gfp_flags) } set_page_private(page, 0); - set_page_refcounted(page); - arch_alloc_page(page, order); kernel_map_pages(page, 1 << order, 1); @@ -876,6 +874,14 @@ static int prep_new_page(struct page *page, int order, gfp_t gfp_flags) if (order && (gfp_flags & __GFP_COMP)) prep_compound_page(page, order); + /* + * Make sure the caller of get_page_unless_zero() will see the + * fully initialized page. Say, to ensure that compound_lock() + * can't race with the non-atomic __SetPage*() above. + */ + smp_wmb(); + set_page_refcounted(page); + return 0; } -- 1.5.5.1