2023-07-21 07:24:06

by Kemeng Shi

[permalink] [raw]
Subject: [PATCH v2 0/2] Two minor cleanups for compaction

v1->v2:
collect reviewed-by from Baolin to patch 2.

This series contains two random cleanups for compaction.
Patch 1 correct wrong comment in fast_isolate_freepages.
Patch 2 avoid to call unneeded pageblock_end_pfn when no_set_skip_hint
is set.
Thanks!

Kemeng Shi (2):
mm/compaction: correct comment of candidate pfn in
fast_isolate_freepages
mm/compaction: avoid unneeded pageblock_end_pfn when no_set_skip_hint
is set

mm/compaction.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

--
2.30.0



2023-07-21 07:24:45

by Kemeng Shi

[permalink] [raw]
Subject: [PATCH v2 1/2] mm/compaction: correct comment of candidate pfn in fast_isolate_freepages

If no preferred one was not found, we will use candidate page with maximum
pfn > min_pfn which is saved in high_pfn. Correct "minimum" to "maximum
candidate" in comment.

Signed-off-by: Kemeng Shi <[email protected]>
---
mm/compaction.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index eb1d3d9a422c..c0d8d08fc163 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1526,7 +1526,7 @@ static void fast_isolate_freepages(struct compact_control *cc)
break;
}

- /* Use a minimum pfn if a preferred one was not found */
+ /* Use a maximum candidate pfn if a preferred one was not found */
if (!page && high_pfn) {
page = pfn_to_page(high_pfn);

--
2.30.0


2023-07-21 07:56:27

by Kemeng Shi

[permalink] [raw]
Subject: [PATCH v2 2/2] mm/compaction: avoid unneeded pageblock_end_pfn when no_set_skip_hint is set

Move pageblock_end_pfn after no_set_skip_hint check to avoid unneeded
pageblock_end_pfn if no_set_skip_hint is set.

Signed-off-by: Kemeng Shi <[email protected]>
Reviewed-by: Baolin Wang <[email protected]>
---
mm/compaction.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index c0d8d08fc163..9b7a0a69e19f 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -458,12 +458,12 @@ static void update_cached_migrate(struct compact_control *cc, unsigned long pfn)
{
struct zone *zone = cc->zone;

- pfn = pageblock_end_pfn(pfn);
-
/* Set for isolation rather than compaction */
if (cc->no_set_skip_hint)
return;

+ pfn = pageblock_end_pfn(pfn);
+
if (pfn > zone->compact_cached_migrate_pfn[0])
zone->compact_cached_migrate_pfn[0] = pfn;
if (cc->mode != MIGRATE_ASYNC &&
--
2.30.0


2023-07-24 12:32:26

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] mm/compaction: avoid unneeded pageblock_end_pfn when no_set_skip_hint is set

On 21.07.23 17:09, Kemeng Shi wrote:
> Move pageblock_end_pfn after no_set_skip_hint check to avoid unneeded
> pageblock_end_pfn if no_set_skip_hint is set.
>
> Signed-off-by: Kemeng Shi <[email protected]>
> Reviewed-by: Baolin Wang <[email protected]>
> ---
> mm/compaction.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index c0d8d08fc163..9b7a0a69e19f 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -458,12 +458,12 @@ static void update_cached_migrate(struct compact_control *cc, unsigned long pfn)
> {
> struct zone *zone = cc->zone;
>
> - pfn = pageblock_end_pfn(pfn);
> -
> /* Set for isolation rather than compaction */
> if (cc->no_set_skip_hint)
> return;
>
> + pfn = pageblock_end_pfn(pfn);
> +
> if (pfn > zone->compact_cached_migrate_pfn[0])
> zone->compact_cached_migrate_pfn[0] = pfn;
> if (cc->mode != MIGRATE_ASYNC &&

I'm pretty sure the compiler can to optimize this -- there should not be
any function call involved, so the compiler can reorder as it pleases.

Anyhow, change itself looks good.

Reviewed-by: David Hildenbrand <[email protected]>

--
Cheers,

David / dhildenb


2023-07-31 04:06:59

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] mm/compaction: correct comment of candidate pfn in fast_isolate_freepages



On 7/21/2023 11:09 PM, Kemeng Shi wrote:
> If no preferred one was not found, we will use candidate page with maximum
> pfn > min_pfn which is saved in high_pfn. Correct "minimum" to "maximum
> candidate" in comment.
>
> Signed-off-by: Kemeng Shi <[email protected]>

Yes, the 'minimum' can be confused. At least to me, it looks good.
Reviewed-by: Baolin Wang <[email protected]>

> ---
> mm/compaction.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index eb1d3d9a422c..c0d8d08fc163 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1526,7 +1526,7 @@ static void fast_isolate_freepages(struct compact_control *cc)
> break;
> }
>
> - /* Use a minimum pfn if a preferred one was not found */
> + /* Use a maximum candidate pfn if a preferred one was not found */
> if (!page && high_pfn) {
> page = pfn_to_page(high_pfn);
>