All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Sonny Rao <sonnyrao@chromium.org>
Cc: linux-kernel@vger.kernel.org,
	Fengguang Wu <fengguang.wu@intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, Mandeep Singh Baines <msb@chromium.org>,
	Johannes Weiner <jweiner@redhat.com>,
	Olof Johansson <olofj@chromium.org>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	Aaron Durbin <adurbin@chromium.org>,
	stable@vger.kernel.org, Puneet Kumar <puneetster@chromium.org>
Subject: Re: [PATCHv4] mm: Fix calculation of dirtyable memory
Date: Mon, 19 Nov 2012 08:40:01 -0500	[thread overview]
Message-ID: <20121119134001.GA2799@cmpxchg.org> (raw)
In-Reply-To: <1352756146-11837-1-git-send-email-sonnyrao@chromium.org>

On Mon, Nov 12, 2012 at 01:35:46PM -0800, Sonny Rao wrote:
> The system uses global_dirtyable_memory() to calculate
> number of dirtyable pages/pages that can be allocated
> to the page cache.  A bug causes an underflow thus making
> the page count look like a big unsigned number.  This in turn
> confuses the dirty writeback throttling to aggressively write
> back pages as they become dirty (usually 1 page at a time).
> This generally only affects systems with highmem because the
> underflowed count gets subtracted from the global count of
> dirtyable memory.
> 
> The problem was introduced with v3.2-4896-gab8fabd
> 
> Fix is to ensure we don't get an underflowed total of either highmem
> or global dirtyable memory.
> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Puneet Kumar <puneetster@chromium.org>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> CC: stable@vger.kernel.org
> ---
>  v2: added apkm's suggestion to make the highmem calculation better
>  v3: added Fengguang Wu's suggestions fix zone_dirtyable_memory() and
>      (offlist mail) to use max() in global_dirtyable_memory()
>  v4: Added suggestions to description clarifying the role of highmem
>       and the commit which originally caused the problem
>  mm/page-writeback.c |   25 ++++++++++++++++++++-----
>  1 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 830893b..f9efbe8 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -201,6 +201,18 @@ static unsigned long highmem_dirtyable_memory(unsigned long total)
>  		     zone_reclaimable_pages(z) - z->dirty_balance_reserve;
>  	}
>  	/*
> +	 * Unreclaimable memory (kernel memory or anonymous memory
> +	 * without swap) can bring down the dirtyable pages below
> +	 * the zone's dirty balance reserve and the above calculation
> +	 * will underflow.  However we still want to add in nodes
> +	 * which are below threshold (negative values) to get a more
> +	 * accurate calculation but make sure that the total never
> +	 * underflows.
> +	 */
> +	if ((long)x < 0)
> +		x = 0;
> +
> +	/*
>  	 * Make sure that the number of highmem pages is never larger
>  	 * than the number of the total dirtyable memory. This can only
>  	 * occur in very strange VM situations but we want to make sure
> @@ -222,8 +234,8 @@ static unsigned long global_dirtyable_memory(void)
>  {
>  	unsigned long x;
>  
> -	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages() -
> -	    dirty_balance_reserve;
> +	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
> +	x -= max(x, dirty_balance_reserve);

This unconditionally zeroes out x, except when it underflows it...

min().

>  	if (!vm_highmem_is_dirtyable)
>  		x -= highmem_dirtyable_memory(x);
> @@ -290,9 +302,12 @@ static unsigned long zone_dirtyable_memory(struct zone *zone)
>  	 * highmem zone can hold its share of dirty pages, so we don't
>  	 * care about vm_highmem_is_dirtyable here.
>  	 */
> -	return zone_page_state(zone, NR_FREE_PAGES) +
> -	       zone_reclaimable_pages(zone) -
> -	       zone->dirty_balance_reserve;
> +	unsigned long nr_pages = zone_page_state(zone, NR_FREE_PAGES) +
> +		zone_reclaimable_pages(zone);
> +
> +	/* don't allow this to underflow */
> +	nr_pages -= max(nr_pages, zone->dirty_balance_reserve);
> +	return nr_pages;

min().

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Sonny Rao <sonnyrao@chromium.org>
Cc: linux-kernel@vger.kernel.org,
	Fengguang Wu <fengguang.wu@intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, Mandeep Singh Baines <msb@chromium.org>,
	Johannes Weiner <jweiner@redhat.com>,
	Olof Johansson <olofj@chromium.org>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	Aaron Durbin <adurbin@chromium.org>,
	stable@vger.kernel.org, Puneet Kumar <puneetster@chromium.org>
Subject: Re: [PATCHv4] mm: Fix calculation of dirtyable memory
Date: Mon, 19 Nov 2012 08:40:01 -0500	[thread overview]
Message-ID: <20121119134001.GA2799@cmpxchg.org> (raw)
In-Reply-To: <1352756146-11837-1-git-send-email-sonnyrao@chromium.org>

On Mon, Nov 12, 2012 at 01:35:46PM -0800, Sonny Rao wrote:
> The system uses global_dirtyable_memory() to calculate
> number of dirtyable pages/pages that can be allocated
> to the page cache.  A bug causes an underflow thus making
> the page count look like a big unsigned number.  This in turn
> confuses the dirty writeback throttling to aggressively write
> back pages as they become dirty (usually 1 page at a time).
> This generally only affects systems with highmem because the
> underflowed count gets subtracted from the global count of
> dirtyable memory.
> 
> The problem was introduced with v3.2-4896-gab8fabd
> 
> Fix is to ensure we don't get an underflowed total of either highmem
> or global dirtyable memory.
> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Puneet Kumar <puneetster@chromium.org>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> CC: stable@vger.kernel.org
> ---
>  v2: added apkm's suggestion to make the highmem calculation better
>  v3: added Fengguang Wu's suggestions fix zone_dirtyable_memory() and
>      (offlist mail) to use max() in global_dirtyable_memory()
>  v4: Added suggestions to description clarifying the role of highmem
>       and the commit which originally caused the problem
>  mm/page-writeback.c |   25 ++++++++++++++++++++-----
>  1 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 830893b..f9efbe8 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -201,6 +201,18 @@ static unsigned long highmem_dirtyable_memory(unsigned long total)
>  		     zone_reclaimable_pages(z) - z->dirty_balance_reserve;
>  	}
>  	/*
> +	 * Unreclaimable memory (kernel memory or anonymous memory
> +	 * without swap) can bring down the dirtyable pages below
> +	 * the zone's dirty balance reserve and the above calculation
> +	 * will underflow.  However we still want to add in nodes
> +	 * which are below threshold (negative values) to get a more
> +	 * accurate calculation but make sure that the total never
> +	 * underflows.
> +	 */
> +	if ((long)x < 0)
> +		x = 0;
> +
> +	/*
>  	 * Make sure that the number of highmem pages is never larger
>  	 * than the number of the total dirtyable memory. This can only
>  	 * occur in very strange VM situations but we want to make sure
> @@ -222,8 +234,8 @@ static unsigned long global_dirtyable_memory(void)
>  {
>  	unsigned long x;
>  
> -	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages() -
> -	    dirty_balance_reserve;
> +	x = global_page_state(NR_FREE_PAGES) + global_reclaimable_pages();
> +	x -= max(x, dirty_balance_reserve);

This unconditionally zeroes out x, except when it underflows it...

min().

>  	if (!vm_highmem_is_dirtyable)
>  		x -= highmem_dirtyable_memory(x);
> @@ -290,9 +302,12 @@ static unsigned long zone_dirtyable_memory(struct zone *zone)
>  	 * highmem zone can hold its share of dirty pages, so we don't
>  	 * care about vm_highmem_is_dirtyable here.
>  	 */
> -	return zone_page_state(zone, NR_FREE_PAGES) +
> -	       zone_reclaimable_pages(zone) -
> -	       zone->dirty_balance_reserve;
> +	unsigned long nr_pages = zone_page_state(zone, NR_FREE_PAGES) +
> +		zone_reclaimable_pages(zone);
> +
> +	/* don't allow this to underflow */
> +	nr_pages -= max(nr_pages, zone->dirty_balance_reserve);
> +	return nr_pages;

min().

  parent reply	other threads:[~2012-11-19 13:40 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-08 23:25 [PATCH] mm: Fix calculation of dirtyable memory Sonny Rao
2012-11-08 23:25 ` Sonny Rao
2012-11-08 23:37 ` Andrew Morton
2012-11-08 23:37   ` Andrew Morton
2012-11-09  0:42   ` [PATCHv2] " Sonny Rao
2012-11-09  0:42     ` Sonny Rao
2012-11-09  0:45     ` Sonny Rao
2012-11-09  0:45       ` Sonny Rao
2012-11-09  0:52       ` Sonny Rao
2012-11-09  0:52         ` Sonny Rao
2012-11-09  2:36         ` Fengguang Wu
2012-11-09  2:36           ` Fengguang Wu
2012-11-12 19:35           ` [PATCH] " Sonny Rao
2012-11-12 19:35             ` Sonny Rao
2012-11-12 20:32             ` Johannes Weiner
2012-11-12 20:32               ` Johannes Weiner
2012-11-12 21:35               ` [PATCHv4] " Sonny Rao
2012-11-12 21:35                 ` Sonny Rao
2012-11-17 20:41                 ` Damien Wyart
2012-11-19 13:40                 ` Johannes Weiner [this message]
2012-11-19 13:40                   ` Johannes Weiner
2012-11-19 18:41                   ` [PATCHv5] " Sonny Rao
2012-11-19 18:41                     ` Sonny Rao
2012-11-19 18:44                     ` Sonny Rao
2012-11-19 18:44                       ` Sonny Rao
2012-11-19 19:22                       ` Damien Wyart
2012-11-19 19:22                         ` Damien Wyart

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=20121119134001.GA2799@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=adurbin@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=fengguang.wu@intel.com \
    --cc=jweiner@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=msb@chromium.org \
    --cc=olofj@chromium.org \
    --cc=puneetster@chromium.org \
    --cc=sonnyrao@chromium.org \
    --cc=stable@vger.kernel.org \
    --cc=wad@chromium.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.