linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Zi Yan <ziy@nvidia.com>
To: Yajun Deng <yajun.deng@linux.dev>
Cc: akpm@linux-foundation.org, mgorman@techsingularity.net,
	david@redhat.com, vbabka@suse.cz, rppt@linux.ibm.com,
	osalvador@suse.de, rppt@kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm/page_alloc: optimize find_suitable_fallback() and fallbacks array
Date: Thu, 09 Feb 2023 10:50:48 -0500	[thread overview]
Message-ID: <4C196D76-49A9-4B06-A51F-D8A13109DF3B@nvidia.com> (raw)
In-Reply-To: <20230209101144.496144-1-yajun.deng@linux.dev>

[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]

On 9 Feb 2023, at 5:11, Yajun Deng wrote:

> There is no need to execute the next loop if it not return in the first
> loop. So add a break at the end of the loop.

Can you explain why? If it is the case, MIGRATE_UNMOVABLE cannot fall back
to MIGRATE_MOVABLE? And MIGRATE_MOVABLE cannot fall back to MIGRATE_UNMOVABLE?
And MIGRATE_RECLAIMABLE cannot fall back to MIGRATE_MOVABLE?

>
> At the same time, add !migratetype_is_mergeable() before the loop and
> reduce the first index size from MIGRATE_TYPES to MIGRATE_PCPTYPES in
> fallbacks array.

You sent a patch: https://lore.kernel.org/all/20230203100132.1627787-1-yajun.deng@linux.dev/T/#u, why not squash this one into that? Why do
we need two separate small patches working on the same code?

Thanks.
>
> Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/mmzone.h |  2 +-
>  mm/page_alloc.c        | 11 +++++------
>  2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index ab94985ee7d9..0a817b8c7fb2 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -85,7 +85,7 @@ static inline bool is_migrate_movable(int mt)
>   * Check whether a migratetype can be merged with another migratetype.
>   *
>   * It is only mergeable when it can fall back to other migratetypes for
> - * allocation. See fallbacks[MIGRATE_TYPES][3] in page_alloc.c.
> + * allocation. See fallbacks[][] array in page_alloc.c.
>   */
>  static inline bool migratetype_is_mergeable(int mt)
>  {
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1113483fa6c5..536e8d838fb5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2603,7 +2603,7 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,
>   *
>   * The other migratetypes do not have fallbacks.
>   */
> -static int fallbacks[MIGRATE_TYPES][MIGRATE_PCPTYPES - 1] = {
> +static int fallbacks[MIGRATE_PCPTYPES][MIGRATE_PCPTYPES - 1] = {
>  	[MIGRATE_UNMOVABLE]   = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE   },
>  	[MIGRATE_MOVABLE]     = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE },
>  	[MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE,   MIGRATE_MOVABLE   },
> @@ -2861,7 +2861,7 @@ int find_suitable_fallback(struct free_area *area, unsigned int order,
>  	int i;
>  	int fallback_mt;
>
> -	if (area->nr_free == 0)
> +	if (area->nr_free == 0 || !migratetype_is_mergeable(migratetype))
>  		return -1;
>
>  	*can_steal = false;
> @@ -2873,11 +2873,10 @@ int find_suitable_fallback(struct free_area *area, unsigned int order,
>  		if (can_steal_fallback(order, migratetype))
>  			*can_steal = true;
>
> -		if (!only_stealable)
> -			return fallback_mt;
> -
> -		if (*can_steal)
> +		if (!only_stealable || *can_steal)
>  			return fallback_mt;
> +		else
> +			break;
>  	}
>
>  	return -1;
> -- 
> 2.25.1

--
Best Regards,
Yan, Zi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]

  reply	other threads:[~2023-02-09 15:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-09 10:11 Yajun Deng
2023-02-09 15:50 ` Zi Yan [this message]
2023-02-10  1:57 ` Yajun Deng
2023-02-10  2:14   ` Zi Yan
2023-02-10  2:33   ` Yajun Deng
2023-02-10  2:51   ` Yajun Deng
2023-02-10  7:58     ` Vlastimil Babka
2023-02-10  8:12     ` Yajun Deng

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=4C196D76-49A9-4B06-A51F-D8A13109DF3B@nvidia.com \
    --to=ziy@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=osalvador@suse.de \
    --cc=rppt@kernel.org \
    --cc=rppt@linux.ibm.com \
    --cc=vbabka@suse.cz \
    --cc=yajun.deng@linux.dev \
    /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