[PATCH] mm/migrate: make migrate_pages_batch() stats consistent.

[PATCH] mm/migrate: make migrate_pages_batch() stats consistent.

[Zi Yan]

From: Zi Yan <ziy@nvidia.com>

As Ying pointed out in [1], stats->nr_thp_failed needs to be updated to
avoid stats inconsistency between MIGRATE_SYNC and MIGRATE_ASYNC when
calling migrate_pages_batch().

[1] https://lore.kernel.org/linux-mm/87msnq7key.fsf@yhuang6-desk2.ccr.corp.intel.com/

Suggested-by: "Huang, Ying" <ying.huang@intel.com>
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 mm/migrate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/migrate.c b/mm/migrate.c
index 2cc5a68f6843..20cb9f5f7446 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1659,6 +1659,10 @@ static int migrate_pages_batch(struct list_head *from,
 			 * migrate_pages() may report success with (split but
 			 * unmigrated) pages still on its fromlist; whereas it
 			 * always reports success when its fromlist is empty.
+			 * stats->nr_thp_failed should be increased too,
+			 * otherwise stats inconsistency will happen when
+			 * migrate_pages_batch is called via migrate_pages()
+			 * with MIGRATE_SYNC and MIGRATE_ASYNC.
 			 *
 			 * Only check it without removing it from the list.
 			 * Since the folio can be on deferred_split_scan()
@@ -1675,6 +1679,7 @@ static int migrate_pages_batch(struct list_head *from,
 			   !list_empty(&folio->_deferred_list)) {
 				if (try_split_folio(folio, split_folios) == 0) {
 					nr_failed++;
+					stats->nr_thp_failed += is_thp;
 					stats->nr_thp_split += is_thp;
 					stats->nr_split++;
 					continue;

base-commit: 14d7c92f8df9c0964ae6f8b813c1b3ac38120825
-- 
2.43.0

Re: [PATCH] mm/migrate: make migrate_pages_batch() stats consistent.

[Andrew Morton]

On Tue, 18 Jun 2024 09:41:51 -0400 Zi Yan <zi.yan@sent.com> wrote:

> From: Zi Yan <ziy@nvidia.com>
> 
> As Ying pointed out in [1], stats->nr_thp_failed needs to be updated to
> avoid stats inconsistency between MIGRATE_SYNC and MIGRATE_ASYNC when
> calling migrate_pages_batch().
> 

Thanks.  I'll add

Fixes: 7262f208ca68 ("mm/migrate: split source folio if it is on deferred split list")

and shall queue it for 6.10-rcX.

Please don't forget such details.

Re: [PATCH] mm/migrate: make migrate_pages_batch() stats consistent.

[Zi Yan]

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

On 18 Jun 2024, at 15:44, Andrew Morton wrote:

> On Tue, 18 Jun 2024 09:41:51 -0400 Zi Yan <zi.yan@sent.com> wrote:
>
>> From: Zi Yan <ziy@nvidia.com>
>>
>> As Ying pointed out in [1], stats->nr_thp_failed needs to be updated to
>> avoid stats inconsistency between MIGRATE_SYNC and MIGRATE_ASYNC when
>> calling migrate_pages_batch().
>>
>
> Thanks.  I'll add
>
> Fixes: 7262f208ca68 ("mm/migrate: split source folio if it is on deferred split list")
>
> and shall queue it for 6.10-rcX.
>
> Please don't forget such details.

I thought about the Fixes tag. Since Hugh's patch fixes the
BUG_ON and VM_BUG_ON, I did not add it here. But yeah stats inconsistency
is something needs to be fixed. Thanks for adding it.

--
Best Regards,
Yan, Zi

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

Re: [PATCH] mm/migrate: make migrate_pages_batch() stats consistent.

[Huang, Ying]

Zi Yan <zi.yan@sent.com> writes:

> From: Zi Yan <ziy@nvidia.com>
>
> As Ying pointed out in [1], stats->nr_thp_failed needs to be updated to
> avoid stats inconsistency between MIGRATE_SYNC and MIGRATE_ASYNC when
> calling migrate_pages_batch().
>
> [1] https://lore.kernel.org/linux-mm/87msnq7key.fsf@yhuang6-desk2.ccr.corp.intel.com/

IMHO, we shouldn't ask people to use link to understand the patch
description or commit message.  If so, we can make the link less
outstanding and maybe add more words about the issue.

> Suggested-by: "Huang, Ying" <ying.huang@intel.com>
> Signed-off-by: Zi Yan <ziy@nvidia.com>

Otherwise, LGTM, Thanks!

[snip]

--
Best Regards,
Huang, Ying

Re: [PATCH] mm/migrate: make migrate_pages_batch() stats consistent.

[Zi Yan]

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

On 19 Jun 2024, at 3:25, Huang, Ying wrote:

> Zi Yan <zi.yan@sent.com> writes:
>
>> From: Zi Yan <ziy@nvidia.com>
>>
>> As Ying pointed out in [1], stats->nr_thp_failed needs to be updated to
>> avoid stats inconsistency between MIGRATE_SYNC and MIGRATE_ASYNC when
>> calling migrate_pages_batch().

Because if not, when migrate_pages_batch() is called via
migrate_pages(MIGRATE_ASYNC), nr_thp_failed will not be increased and
when migrate_pages_batch() is called via migrate_pages(MIGRATE_SYNC*),
nr_thp_failed will be increase in migrate_pages_sync() by
stats->nr_thp_failed += astats.nr_thp_split.

>>
>> [1] https://lore.kernel.org/linux-mm/87msnq7key.fsf@yhuang6-desk2.ccr.corp.intel.com/
>
> IMHO, we shouldn't ask people to use link to understand the patch
> description or commit message.  If so, we can make the link less
> outstanding and maybe add more words about the issue.

I summarized your explanation from the link. You think the summary is not
enough? I can add more text. Let me know if the above text looks
good to you, then I can resend the patch. Thanks.

--
Best Regards,
Yan, Zi

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

Re: [PATCH] mm/migrate: make migrate_pages_batch() stats consistent.

[Huang, Ying]

Zi Yan <ziy@nvidia.com> writes:

> On 19 Jun 2024, at 3:25, Huang, Ying wrote:
>
>> Zi Yan <zi.yan@sent.com> writes:
>>
>>> From: Zi Yan <ziy@nvidia.com>
>>>
>>> As Ying pointed out in [1], stats->nr_thp_failed needs to be updated to
>>> avoid stats inconsistency between MIGRATE_SYNC and MIGRATE_ASYNC when
>>> calling migrate_pages_batch().
>
> Because if not, when migrate_pages_batch() is called via
> migrate_pages(MIGRATE_ASYNC), nr_thp_failed will not be increased and
> when migrate_pages_batch() is called via migrate_pages(MIGRATE_SYNC*),
> nr_thp_failed will be increase in migrate_pages_sync() by
> stats->nr_thp_failed += astats.nr_thp_split.
>
>>>
>>> [1] https://lore.kernel.org/linux-mm/87msnq7key.fsf@yhuang6-desk2.ccr.corp.intel.com/
>>
>> IMHO, we shouldn't ask people to use link to understand the patch
>> description or commit message.  If so, we can make the link less
>> outstanding and maybe add more words about the issue.
>
> I summarized your explanation from the link. You think the summary is not
> enough? I can add more text. Let me know if the above text looks
> good to you, then I can resend the patch. Thanks.

It looks good to me, Thanks!

--
Best Regards,
Huang, Ying