All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Chen Feng <puck.chen@hisilicon.com>,
	akpm@linux-foundation.org, iamjoonsoo.kim@lge.com,
	mina86@mina86.com, rientjes@google.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Cc: xuyiping@hisilicon.com, suzhuangluan@hisilicon.com,
	dan.zhao@hisilicon.com, qijiwen@hisilicon.com,
	oliver.fu@hisilicon.com, puck.chen@foxmail.com
Subject: Re: [PATCH] mm: compact: remove watermark check at compact suitable
Date: Mon, 23 May 2016 10:20:22 +0200	[thread overview]
Message-ID: <5742BD46.8050403@suse.cz> (raw)
In-Reply-To: <1463973617-10599-1-git-send-email-puck.chen@hisilicon.com>

On 05/23/2016 05:20 AM, Chen Feng wrote:
> There are two paths calling this function.
> For direct compact, there is no need to check the zone watermark here.
> For kswapd wakeup kcompactd, since there is a reclaim before this.
> It makes sense to do compact even the watermark is ok at this time.

Hi,

I'm just working on v2 of the series [1] and some patches planned for v2 are 
trying to simplify the watermark checks around compaction. The check you are 
removing looked like simple and obvious one, so I didn't change it. But I'll 
think more about your patch, e.g. if there are some corner cases. See for 
example the fragindex check:

          * index of -1000 would imply allocations might succeed depending on
          * watermarks, but we already failed the high-order watermark check

After your patch, there is no more high-order watermark check, so the assumption 
here is gone.
Also the comment above __compaction_suitable() should be updated too.

[1] http://lkml.kernel.org/r/<1462865763-22084-1-git-send-email-vbabka@suse.cz>

> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> ---
>   mm/compaction.c | 7 -------
>   1 file changed, 7 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 8fa2540..cb322df 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1260,13 +1260,6 @@ static unsigned long __compaction_suitable(struct zone *zone, int order,
>   		return COMPACT_CONTINUE;
>
>   	watermark = low_wmark_pages(zone);
> -	/*
> -	 * If watermarks for high-order allocation are already met, there
> -	 * should be no need for compaction at all.
> -	 */
> -	if (zone_watermark_ok(zone, order, watermark, classzone_idx,
> -								alloc_flags))
> -		return COMPACT_PARTIAL;
>
>   	/*
>   	 * Watermarks for order-0 must be met for compaction. Note the 2UL.
>

--
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: Vlastimil Babka <vbabka@suse.cz>
To: Chen Feng <puck.chen@hisilicon.com>,
	akpm@linux-foundation.org, iamjoonsoo.kim@lge.com,
	mina86@mina86.com, rientjes@google.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Cc: xuyiping@hisilicon.com, suzhuangluan@hisilicon.com,
	dan.zhao@hisilicon.com, qijiwen@hisilicon.com,
	oliver.fu@hisilicon.com, puck.chen@foxmail.com
Subject: Re: [PATCH] mm: compact: remove watermark check at compact suitable
Date: Mon, 23 May 2016 10:20:22 +0200	[thread overview]
Message-ID: <5742BD46.8050403@suse.cz> (raw)
In-Reply-To: <1463973617-10599-1-git-send-email-puck.chen@hisilicon.com>

On 05/23/2016 05:20 AM, Chen Feng wrote:
> There are two paths calling this function.
> For direct compact, there is no need to check the zone watermark here.
> For kswapd wakeup kcompactd, since there is a reclaim before this.
> It makes sense to do compact even the watermark is ok at this time.

Hi,

I'm just working on v2 of the series [1] and some patches planned for v2 are 
trying to simplify the watermark checks around compaction. The check you are 
removing looked like simple and obvious one, so I didn't change it. But I'll 
think more about your patch, e.g. if there are some corner cases. See for 
example the fragindex check:

          * index of -1000 would imply allocations might succeed depending on
          * watermarks, but we already failed the high-order watermark check

After your patch, there is no more high-order watermark check, so the assumption 
here is gone.
Also the comment above __compaction_suitable() should be updated too.

[1] http://lkml.kernel.org/r/<1462865763-22084-1-git-send-email-vbabka@suse.cz>

> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> ---
>   mm/compaction.c | 7 -------
>   1 file changed, 7 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 8fa2540..cb322df 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1260,13 +1260,6 @@ static unsigned long __compaction_suitable(struct zone *zone, int order,
>   		return COMPACT_CONTINUE;
>
>   	watermark = low_wmark_pages(zone);
> -	/*
> -	 * If watermarks for high-order allocation are already met, there
> -	 * should be no need for compaction at all.
> -	 */
> -	if (zone_watermark_ok(zone, order, watermark, classzone_idx,
> -								alloc_flags))
> -		return COMPACT_PARTIAL;
>
>   	/*
>   	 * Watermarks for order-0 must be met for compaction. Note the 2UL.
>

  reply	other threads:[~2016-05-23  8:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-23  3:20 [PATCH] mm: compact: remove watermark check at compact suitable Chen Feng
2016-05-23  3:20 ` Chen Feng
2016-05-23  8:20 ` Vlastimil Babka [this message]
2016-05-23  8:20   ` Vlastimil Babka

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=5742BD46.8050403@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=dan.zhao@hisilicon.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mina86@mina86.com \
    --cc=oliver.fu@hisilicon.com \
    --cc=puck.chen@foxmail.com \
    --cc=puck.chen@hisilicon.com \
    --cc=qijiwen@hisilicon.com \
    --cc=rientjes@google.com \
    --cc=suzhuangluan@hisilicon.com \
    --cc=xuyiping@hisilicon.com \
    /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.