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

From: Michal Hocko
Date: Fri May 20 2016 - 09:06:56 EST


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@xxxxxxxxxx>

Acked-by: Michal Hocko <mhocko@xxxxxxxx>

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