linux-kernel - Re: [PATCH] mm: move_pages: fix the return value if there are not-migrated pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 20 Jan 2020 14:06:24 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Yang Shi <yang.shi@...ux.alibaba.com>
Cc:     richardw.yang@...ux.intel.com, akpm@...ux-foundation.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH] mm: move_pages: fix the return value if there are
 not-migrated pages

On Sat 18-01-20 13:26:43, Yang Shi wrote:
> The do_move_pages_to_node() might return > 0 value, the number of pages
> that are not migrated, then the value will be returned to userspace
> directly.  But, move_pages() syscall would just return 0 or errno.  So,
> we need reset the return value to 0 for such case as what pre-v4.17 did.

The patch is wrong. migrate_pages returns the number of pages it
_hasn't_ migrated or -errno. Yeah that semantic sucks but...
So err != 0 is always an error. Except err > 0 doesn't really provide
any useful information to the userspace. I cannot really remember what
was the actual behavior before my rework because there were some gotchas
hidden there.

If you want to fix this properly then you have to query node status of
each page unmigrated when migrate_pages fails with > 0. This would be
easier if the fix is done on the latest cleanup posted to the list which
consolidates all do_move_pages_to_node and store_status calls to a
single function.

> Fixes: a49bd4d71637 ("mm, numa: rework do_pages_move")
> Cc: Michal Hocko <mhocko@...e.com>
> Cc: Wei Yang <richardw.yang@...ux.intel.com>
> Cc: <stable@...r.kernel.org>    [4.17+]
> Signed-off-by: Yang Shi <yang.shi@...ux.alibaba.com>
> ---
>  mm/migrate.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 86873b6..3e75432 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1659,8 +1659,11 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  			goto out_flush;
>  
>  		err = do_move_pages_to_node(mm, &pagelist, current_node);
> -		if (err)
> +		if (err) {
> +			if (err > 0)
> +				err = 0;
>  			goto out;
> +		}
>  		if (i > start) {
>  			err = store_status(status, start, current_node, i - start);
>  			if (err)
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs