From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752502AbcERHwC (ORCPT ); Wed, 18 May 2016 03:52:02 -0400 Received: from mx2.suse.de ([195.135.220.15]:51512 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751466AbcERHwA (ORCPT ); Wed, 18 May 2016 03:52:00 -0400 Subject: Re: [PATCH 28/28] mm, page_alloc: Defer debugging checks of pages allocated from the PCP To: Naoya Horiguchi , Mel Gorman References: <1460710760-32601-1-git-send-email-mgorman@techsingularity.net> <1460711275-1130-1-git-send-email-mgorman@techsingularity.net> <1460711275-1130-16-git-send-email-mgorman@techsingularity.net> <20160517064153.GA23930@hori1.linux.bs1.fc.nec.co.jp> Cc: Andrew Morton , Jesper Dangaard Brouer , Linux-MM , LKML From: Vlastimil Babka Message-ID: <573C1F1E.4040201@suse.cz> Date: Wed, 18 May 2016 09:51:58 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <20160517064153.GA23930@hori1.linux.bs1.fc.nec.co.jp> Content-Type: text/plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/17/2016 08:41 AM, Naoya Horiguchi wrote: >> @@ -2579,20 +2612,22 @@ struct page *buffered_rmqueue(struct zone *preferred_zone, >> struct list_head *list; >> >> local_irq_save(flags); >> - pcp = &this_cpu_ptr(zone->pageset)->pcp; >> - list = &pcp->lists[migratetype]; >> - if (list_empty(list)) { >> - pcp->count += rmqueue_bulk(zone, 0, >> - pcp->batch, list, >> - migratetype, cold); >> - if (unlikely(list_empty(list))) >> - goto failed; >> - } >> + do { >> + pcp = &this_cpu_ptr(zone->pageset)->pcp; >> + list = &pcp->lists[migratetype]; >> + if (list_empty(list)) { >> + pcp->count += rmqueue_bulk(zone, 0, >> + pcp->batch, list, >> + migratetype, cold); >> + if (unlikely(list_empty(list))) >> + goto failed; >> + } >> >> - if (cold) >> - page = list_last_entry(list, struct page, lru); >> - else >> - page = list_first_entry(list, struct page, lru); >> + if (cold) >> + page = list_last_entry(list, struct page, lru); >> + else >> + page = list_first_entry(list, struct page, lru); >> + } while (page && check_new_pcp(page)); > > This causes infinite loop when check_new_pcp() returns 1, because the bad > page is still in the list (I assume that a bad page never disappears). > The original kernel is free from this problem because we do retry after > list_del(). So moving the following 3 lines into this do-while block solves > the problem? > > __dec_zone_state(zone, NR_ALLOC_BATCH); > list_del(&page->lru); > pcp->count--; > > There seems no infinit loop issue in order > 0 block below, because bad pages > are deleted from free list in __rmqueue_smallest(). Ooops, thanks for catching this, wish it was sooner... ----8<---- >>From f52f5e2a7dd65f2814183d8fd254ace43120b828 Mon Sep 17 00:00:00 2001 From: Vlastimil Babka Date: Wed, 18 May 2016 09:41:01 +0200 Subject: [PATCH] mm, page_alloc: prevent infinite loop in buffered_rmqueue() In DEBUG_VM kernel, we can hit infinite loop for order == 0 in buffered_rmqueue() when check_new_pcp() returns 1, because the bad page is never removed from the pcp list. Fix this by removing the page before retrying. Also we don't need to check if page is non-NULL, because we simply grab it from the list which was just tested for being non-empty. Fixes: http://www.ozlabs.org/~akpm/mmotm/broken-out/mm-page_alloc-defer-debugging-checks-of-freed-pages-until-a-pcp-drain.patch Reported-by: Naoya Horiguchi Signed-off-by: Vlastimil Babka --- mm/page_alloc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 8c81e2e7b172..d5b93e5dd697 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2641,11 +2641,12 @@ struct page *buffered_rmqueue(struct zone *preferred_zone, page = list_last_entry(list, struct page, lru); else page = list_first_entry(list, struct page, lru); - } while (page && check_new_pcp(page)); - __dec_zone_state(zone, NR_ALLOC_BATCH); - list_del(&page->lru); - pcp->count--; + __dec_zone_state(zone, NR_ALLOC_BATCH); + list_del(&page->lru); + pcp->count--; + + } while (check_new_pcp(page)); } else { /* * We most definitely don't want callers attempting to -- 2.8.2 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f200.google.com (mail-lb0-f200.google.com [209.85.217.200]) by kanga.kvack.org (Postfix) with ESMTP id ED4AA6B007E for ; Wed, 18 May 2016 03:52:00 -0400 (EDT) Received: by mail-lb0-f200.google.com with SMTP id ga2so19670409lbc.0 for ; Wed, 18 May 2016 00:52:00 -0700 (PDT) Received: from mx2.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id l14si9393824wmb.12.2016.05.18.00.51.59 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 18 May 2016 00:51:59 -0700 (PDT) Subject: Re: [PATCH 28/28] mm, page_alloc: Defer debugging checks of pages allocated from the PCP References: <1460710760-32601-1-git-send-email-mgorman@techsingularity.net> <1460711275-1130-1-git-send-email-mgorman@techsingularity.net> <1460711275-1130-16-git-send-email-mgorman@techsingularity.net> <20160517064153.GA23930@hori1.linux.bs1.fc.nec.co.jp> From: Vlastimil Babka Message-ID: <573C1F1E.4040201@suse.cz> Date: Wed, 18 May 2016 09:51:58 +0200 MIME-Version: 1.0 In-Reply-To: <20160517064153.GA23930@hori1.linux.bs1.fc.nec.co.jp> Content-Type: text/plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Naoya Horiguchi , Mel Gorman Cc: Andrew Morton , Jesper Dangaard Brouer , Linux-MM , LKML On 05/17/2016 08:41 AM, Naoya Horiguchi wrote: >> @@ -2579,20 +2612,22 @@ struct page *buffered_rmqueue(struct zone *preferred_zone, >> struct list_head *list; >> >> local_irq_save(flags); >> - pcp = &this_cpu_ptr(zone->pageset)->pcp; >> - list = &pcp->lists[migratetype]; >> - if (list_empty(list)) { >> - pcp->count += rmqueue_bulk(zone, 0, >> - pcp->batch, list, >> - migratetype, cold); >> - if (unlikely(list_empty(list))) >> - goto failed; >> - } >> + do { >> + pcp = &this_cpu_ptr(zone->pageset)->pcp; >> + list = &pcp->lists[migratetype]; >> + if (list_empty(list)) { >> + pcp->count += rmqueue_bulk(zone, 0, >> + pcp->batch, list, >> + migratetype, cold); >> + if (unlikely(list_empty(list))) >> + goto failed; >> + } >> >> - if (cold) >> - page = list_last_entry(list, struct page, lru); >> - else >> - page = list_first_entry(list, struct page, lru); >> + if (cold) >> + page = list_last_entry(list, struct page, lru); >> + else >> + page = list_first_entry(list, struct page, lru); >> + } while (page && check_new_pcp(page)); > > This causes infinite loop when check_new_pcp() returns 1, because the bad > page is still in the list (I assume that a bad page never disappears). > The original kernel is free from this problem because we do retry after > list_del(). So moving the following 3 lines into this do-while block solves > the problem? > > __dec_zone_state(zone, NR_ALLOC_BATCH); > list_del(&page->lru); > pcp->count--; > > There seems no infinit loop issue in order > 0 block below, because bad pages > are deleted from free list in __rmqueue_smallest(). Ooops, thanks for catching this, wish it was sooner... ----8<----