linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@linux.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Linux-MM <linux-mm@kvack.org>,
	Linux-Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] mm, page_alloc: Keep pcp count and list contents in sync if struct page is corrupted
Date: Fri, 2 Dec 2016 09:12:17 +0100	[thread overview]
Message-ID: <20161202081216.GA6830@dhcp22.suse.cz> (raw)
In-Reply-To: <20161202002244.18453-2-mgorman@techsingularity.net>

On Fri 02-12-16 00:22:43, Mel Gorman wrote:
> Vlastimil Babka pointed out that commit 479f854a207c ("mm, page_alloc:
> defer debugging checks of pages allocated from the PCP") will allow the
> per-cpu list counter to be out of sync with the per-cpu list contents
> if a struct page is corrupted. This patch keeps the accounting in sync.
>
> Fixes: 479f854a207c ("mm, page_alloc: defer debugging checks of pages allocated from the PCP")
> Signed-off-by: Mel Gorman <mgorman@suse.de>
> cc: stable@vger.kernel.org [4.7+]

I am trying to think about what would happen if we did go out of sync
and cannot spot a problem. Vlastimil has mentioned something about
free_pcppages_bulk looping for ever but I cannot see it happening right
now. So why is this worth stable backport?

Anyway the patch looks correct
Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/page_alloc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6de9440e3ae2..777ed59570df 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2192,7 +2192,7 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
>  			unsigned long count, struct list_head *list,
>  			int migratetype, bool cold)
>  {
> -	int i;
> +	int i, alloced = 0;
>  
>  	spin_lock(&zone->lock);
>  	for (i = 0; i < count; ++i) {
> @@ -2217,13 +2217,14 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
>  		else
>  			list_add_tail(&page->lru, list);
>  		list = &page->lru;
> +		alloced++;
>  		if (is_migrate_cma(get_pcppage_migratetype(page)))
>  			__mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
>  					      -(1 << order));
>  	}
>  	__mod_zone_page_state(zone, NR_FREE_PAGES, -(i << order));

I guess this deserves a comment (i vs. alloced is confusing and I bet
somebody will come up with a cleanup...). We leak corrupted pages
intentionally so we should uncharge them from the NR_FREE_PAGES.

>  	spin_unlock(&zone->lock);
> -	return i;
> +	return alloced;
>  }
>  
>  #ifdef CONFIG_NUMA
> -- 
> 2.10.2
> 

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2016-12-02  8:12 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-02  0:22 [PATCH 0/2] High-order per-cpu cache v5 Mel Gorman
2016-12-02  0:22 ` [PATCH 1/2] mm, page_alloc: Keep pcp count and list contents in sync if struct page is corrupted Mel Gorman
2016-12-02  3:47   ` Hillf Danton
2016-12-02  6:19     ` Vlastimil Babka
2016-12-02  9:30       ` Hillf Danton
2016-12-02 10:04         ` Michal Hocko
2016-12-02 11:02           ` Mel Gorman
2016-12-02  8:12   ` Michal Hocko [this message]
2016-12-02  9:49     ` Mel Gorman
2016-12-02 10:03       ` Michal Hocko
2016-12-02  0:22 ` [PATCH 2/2] mm: page_alloc: High-order per-cpu page allocator v5 Mel Gorman
2016-12-02  6:03   ` Joonsoo Kim
2016-12-02  8:21     ` Michal Hocko
2016-12-05  3:10       ` Joonsoo Kim
2016-12-02  9:04     ` Mel Gorman
2016-12-05  3:06       ` Joonsoo Kim
2016-12-05  9:57         ` Mel Gorman
2016-12-06  2:43           ` Joonsoo Kim
2016-12-06 13:53             ` Mel Gorman
2016-12-02  8:25   ` Michal Hocko
  -- strict thread matches above, loose matches on Subject: below --
2016-12-02 11:29 [PATCH 0/2] High-order per-cpu cache v6 Mel Gorman
2016-12-02 11:29 ` [PATCH 1/2] mm, page_alloc: Keep pcp count and list contents in sync if struct page is corrupted Mel Gorman
2016-12-02 11:53   ` Vlastimil Babka
2016-12-02 13:15   ` Michal Hocko
2016-12-05  3:39   ` Hillf Danton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161202081216.GA6830@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=brouer@redhat.com \
    --cc=cl@linux.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).