From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751555AbbJLHio (ORCPT ); Mon, 12 Oct 2015 03:38:44 -0400 Received: from mx2.suse.de ([195.135.220.15]:39266 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751149AbbJLHin (ORCPT ); Mon, 12 Oct 2015 03:38:43 -0400 Subject: Re: [RFC] mm: fix a BUG, the page is allocated 2 times To: yalin wang , akpm@linux-foundation.org, mgorman@techsingularity.net, mhocko@suse.com, rientjes@google.com, js1304@gmail.com, kirill.shutemov@linux.intel.com, hannes@cmpxchg.org, alexander.h.duyck@redhat.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <1444617606-8685-1-git-send-email-yalin.wang2010@gmail.com> From: Vlastimil Babka Message-ID: <561B6379.2070407@suse.cz> Date: Mon, 12 Oct 2015 09:38:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <1444617606-8685-1-git-send-email-yalin.wang2010@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12/2015 04:40 AM, yalin wang wrote: > Remove unlikely(order), because we are sure order is not zero if > code reach here, also add if (page == NULL), only allocate page again if > __rmqueue_smallest() failed or alloc_flags & ALLOC_HARDER == 0 The second mentioned change is actually more important as it removes a memory leak! Thanks for catching this. The problem is in patch mm-page_alloc-reserve-pageblocks-for-high-order-atomic-allocations-on-demand.patch and seems to have been due to a change in the last submitted version to make sure the tracepoint is called. > Signed-off-by: yalin wang > --- > mm/page_alloc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 0d6f540..de82e2c 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -2241,13 +2241,13 @@ struct page *buffered_rmqueue(struct zone *preferred_zone, > spin_lock_irqsave(&zone->lock, flags); > > page = NULL; > - if (unlikely(order) && (alloc_flags & ALLOC_HARDER)) { > + if (alloc_flags & ALLOC_HARDER) { > page = __rmqueue_smallest(zone, order, MIGRATE_HIGHATOMIC); > if (page) > trace_mm_page_alloc_zone_locked(page, order, migratetype); > } > - > - page = __rmqueue(zone, order, migratetype, gfp_flags); > + if (page == NULL) "if (!page)" is more common and already used below. We could skip the check for !page in case we don't go through the ALLOC_HARDER branch, but I guess it's not worth the goto, and hopefully the compiler is smart enough anyway... > + page = __rmqueue(zone, order, migratetype, gfp_flags); > spin_unlock(&zone->lock); > if (!page) > goto failed; >