Re: [patch] mm, migrate: increment fail count on ENOMEM

[Michal Hocko <mhocko@kernel.org>]

On Thu 19-05-16 15:11:23, David Rientjes wrote:
> If page migration fails due to -ENOMEM, nr_failed should still be
> incremented for proper statistics.
> 
> This was encountered recently when all page migration vmstats showed 0,
> and inferred that migrate_pages() was never called, although in reality
> the first page migration failed because compaction_alloc() failed to find
> a migration target.
> 
> This patch increments nr_failed so the vmstat is properly accounted on
> ENOMEM.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>

Acked-by: Michal Hocko <mhocko@suse.com>

One question though

> ---
>  mm/migrate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1171,6 +1171,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
>  
>  			switch(rc) {
>  			case -ENOMEM:
> +				nr_failed++;
>  				goto out;
>  			case -EAGAIN:
>  				retry++;

Why don't we need also to count also retries?
---
diff --git a/mm/migrate.c b/mm/migrate.c
index 53ab6398e7a2..ef9c5211ae3c 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1190,9 +1190,9 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
 			}
 		}
 	}
+out:
 	nr_failed += retry;
 	rc = nr_failed;
-out:
 	if (nr_succeeded)
 		count_vm_events(PGMIGRATE_SUCCESS, nr_succeeded);
 	if (nr_failed)
-- 
Michal Hocko
SUSE Labs