linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Rik van Riel <riel@redhat.com>, Vlastimil Babka <vbabka@suse.cz>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Michal Hocko <mhocko@kernel.org>, Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 10/10] mm, page_alloc: Only enforce watermarks for order-0 allocations
Date: Wed, 30 Sep 2015 09:46:50 +0100	[thread overview]
Message-ID: <20150930084650.GM3068@techsingularity.net> (raw)
In-Reply-To: <20150929140507.82b5e02f300038e4bb5b2493@linux-foundation.org>

On Tue, Sep 29, 2015 at 02:05:07PM -0700, Andrew Morton wrote:
> >  static bool __zone_watermark_ok(struct zone *z, unsigned int order,
> >  			unsigned long mark, int classzone_idx, int alloc_flags,
> > @@ -2317,7 +2319,7 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
> >  {
> >  	long min = mark;
> >  	int o;
> > -	long free_cma = 0;
> > +	const bool alloc_harder = (alloc_flags & ALLOC_HARDER);
> 
> hmpf.  Setting a bool to 0x10 is a bit grubby.
>   

Should be safe, but I see your point. For any other type it would be
truncated and look like a bug.

> >  	/* free_pages may go negative - that's OK */
> >  	free_pages -= (1 << order) - 1;
> > @@ -2330,7 +2332,7 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
> >  	 * the high-atomic reserves. This will over-estimate the size of the
> >  	 * atomic reserve but it avoids a search.
> >  	 */
> > -	if (likely(!(alloc_flags & ALLOC_HARDER)))
> > +	if (likely(!alloc_harder))
> >  		free_pages -= z->nr_reserved_highatomic;
> >  	else
> >  		min -= min / 4;
> > @@ -2338,22 +2340,43 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
> >  #ifdef CONFIG_CMA
> >  	/* If allocation can't use CMA areas don't use free CMA pages */
> >  	if (!(alloc_flags & ALLOC_CMA))
> > -		free_cma = zone_page_state(z, NR_FREE_CMA_PAGES);
> > +		free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
> >  #endif
> >  
> > -	if (free_pages - free_cma <= min + z->lowmem_reserve[classzone_idx])
> > +	if (free_pages <= min + z->lowmem_reserve[classzone_idx])
> >  		return false;
> > -	for (o = 0; o < order; o++) {
> > -		/* At the next order, this order's pages become unavailable */
> > -		free_pages -= z->free_area[o].nr_free << o;
> >  
> > -		/* Require fewer higher order pages to be free */
> > -		min >>= 1;
> > +	/* order-0 watermarks are ok */
> 
> because?
> 

The wizard of oz because because!

This should fix it up better than clicking my shoes three times.

---8<---
From: Mel Gorman <mgorman@techsingularity.net>
Subject: [PATCH] mm, page_alloc: only enforce watermarks for order-0
 allocations -fix

This patch is updating comments for clarity and converts a bool to an
int. The code as-is is ok as the compiler is meant to cast it correctly
but it looks odd to people who know the value would be truncated and
lost for other types.

This is a fix to the mmotm patch
mm-page_alloc-only-enforce-watermarks-for-order-0-allocations.patch

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 mm/page_alloc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 25731624d734..fedec98aafca 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2332,7 +2332,7 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
 {
 	long min = mark;
 	int o;
-	const bool alloc_harder = (alloc_flags & ALLOC_HARDER);
+	const int alloc_harder = (alloc_flags & ALLOC_HARDER);
 
 	/* free_pages may go negative - that's OK */
 	free_pages -= (1 << order) - 1;
@@ -2356,14 +2356,19 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
 		free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
 #endif
 
+	/*
+	 * Check watermarks for an order-0 allocation request. If these
+	 * are not met, then a high-order request also cannot go ahead
+	 * even if a suitable page happened to be free.
+	 */
 	if (free_pages <= min + z->lowmem_reserve[classzone_idx])
 		return false;
 
-	/* order-0 watermarks are ok */
+	/* If this is an order-0 request then the watermark is fine */
 	if (!order)
 		return true;
 
-	/* Check at least one high-order page is free */
+	/* For a high-order request, check at least one suitable page is free */
 	for (o = order; o < MAX_ORDER; o++) {
 		struct free_area *area = &z->free_area[o];
 		int mt;

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

  reply	other threads:[~2015-09-30  8:46 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-21 10:52 [PATCH 00/10] Remove zonelist cache and high-order watermark checking v4 Mel Gorman
2015-09-21 10:52 ` [PATCH 01/10] mm, page_alloc: Remove unnecessary parameter from zone_watermark_ok_safe Mel Gorman
2015-09-24 20:01   ` Johannes Weiner
2015-09-21 10:52 ` [PATCH 02/10] mm, page_alloc: Remove unnecessary recalculations for dirty zone balancing Mel Gorman
2015-09-24 20:05   ` Johannes Weiner
2015-09-21 10:52 ` [PATCH 03/10] mm, page_alloc: Remove unnecessary taking of a seqlock when cpusets are disabled Mel Gorman
2015-09-24 20:06   ` Johannes Weiner
2015-09-30 22:22   ` David Rientjes
2015-10-01  7:35     ` Vlastimil Babka
2015-09-21 10:52 ` [PATCH 04/10] mm, page_alloc: Use masks and shifts when converting GFP flags to migrate types Mel Gorman
2015-09-24 20:34   ` Johannes Weiner
2015-09-25 12:50     ` Mel Gorman
2015-09-25 13:56       ` Johannes Weiner
2015-09-21 10:52 ` [PATCH 05/10] mm, page_alloc: Distinguish between being unable to sleep, unwilling to sleep and avoiding waking kswapd Mel Gorman
2015-09-24 13:51   ` Michal Hocko
2015-09-24 20:55   ` Johannes Weiner
2015-09-25 12:51     ` Mel Gorman
2015-09-25 19:01       ` Johannes Weiner
2015-09-29 13:35         ` Mel Gorman
2015-09-30 12:26           ` Vlastimil Babka
2015-09-30 13:17             ` Mel Gorman
2015-10-01  3:04             ` Drokin, Oleg
2015-10-02 12:30               ` Mel Gorman
2015-09-21 10:52 ` [PATCH 06/10] mm, page_alloc: Rename __GFP_WAIT to __GFP_RECLAIM Mel Gorman
2015-09-25 19:03   ` Johannes Weiner
2015-09-28 23:55   ` Andrew Morton
2015-09-29 13:37     ` Mel Gorman
2015-10-01  8:39       ` Vlastimil Babka
2015-10-02 13:03         ` [PATCH] mm: page_alloc: Hide some GFP internals and document the bits and flag combinations -fix Mel Gorman
2015-10-01 14:06       ` [PATCH 06/10] mm, page_alloc: Rename __GFP_WAIT to __GFP_RECLAIM Michal Hocko
2015-09-30 22:25   ` David Rientjes
2015-09-21 10:52 ` [PATCH 07/10] mm, page_alloc: Delete the zonelist_cache Mel Gorman
2015-09-25 19:09   ` Johannes Weiner
2015-09-21 10:52 ` [PATCH 08/10] mm, page_alloc: Remove MIGRATE_RESERVE Mel Gorman
2015-09-21 10:52 ` [PATCH 09/10] mm, page_alloc: Reserve pageblocks for high-order atomic allocations on demand Mel Gorman
2015-09-24 13:50   ` Michal Hocko
2015-09-25 19:22   ` Johannes Weiner
2015-09-29 21:01   ` Andrew Morton
2015-09-30  8:27     ` Mel Gorman
2015-09-30 14:02       ` Vlastimil Babka
2015-09-21 12:03 ` [PATCH 10/10] mm, page_alloc: Only enforce watermarks for order-0 allocations Mel Gorman
2015-09-25 19:32   ` Johannes Weiner
2015-09-29 21:05   ` Andrew Morton
2015-09-30  8:46     ` Mel Gorman [this message]
2015-09-30 14:17       ` Vlastimil Babka
2015-09-30 15:12         ` Mel Gorman
2015-09-30 20:37           ` Andrew Morton
2015-09-30 14:11   ` Vlastimil Babka
  -- strict thread matches above, loose matches on Subject: below --
2015-08-12 10:45 [PATCH 00/10] Remove zonelist cache and high-order watermark checking v2 Mel Gorman
2015-08-12 10:45 ` [PATCH 10/10] mm, page_alloc: Only enforce watermarks for order-0 allocations Mel Gorman
2015-07-20  8:00 [RFC PATCH 00/10] Remove zonelist cache and high-order watermark checking Mel Gorman
2015-07-20  8:00 ` [PATCH 10/10] mm, page_alloc: Only enforce watermarks for order-0 allocations Mel Gorman
2015-07-29 12:25   ` Vlastimil Babka
2015-07-29 13:04     ` Mel Gorman
2015-07-31  6:08   ` Joonsoo Kim
2015-07-31  7:19     ` Mel Gorman
2015-07-31  8:40       ` Joonsoo Kim

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=20150930084650.GM3068@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --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).