linux-kernel - Re: [PATCH 7/8] mm/migrate.c: move page on next iteration

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 11:02:03 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Wei Yang <richardw.yang@...ux.intel.com>
Cc:     akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, yang.shi@...ux.alibaba.com
Subject: Re: [PATCH 7/8] mm/migrate.c: move page on next iteration

On Sun 19-01-20 11:06:35, Wei Yang wrote:
> When page is not successfully queued for migration, we would move pages
> on pagelist immediately. Actually, this could be done in the next
> iteration by telling it we need some help.
> 
> This patch adds a new variable need_move to be an indication. After
> this, we only need to call move_pages_and_store_status() twice.

No! Not another iterator. There are two and this makes the function
quite hard to follow already. We do not want to add a third one.

> Signed-off-by: Wei Yang <richardw.yang@...ux.intel.com>
> ---
>  mm/migrate.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index aee5aeb082c4..2a857fec65b6 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1610,6 +1610,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  	LIST_HEAD(pagelist);
>  	int start, i;
>  	int err = 0, err1;
> +	int need_move = 0;
>  
>  	migrate_prep();
>  
> @@ -1641,13 +1642,15 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  		if (current_node == NUMA_NO_NODE) {
>  			current_node = node;
>  			start = i;
> -		} else if (node != current_node) {
> +		} else if (node != current_node || need_move) {
>  			err = move_pages_and_store_status(mm, current_node,
> -					&pagelist, status, start, i - start);
> +					&pagelist, status, start,
> +					i - start - need_move);
>  			if (err)
>  				goto out;
>  			start = i;
>  			current_node = node;
> +			need_move = 0;
>  		}
>  
>  		/*
> @@ -1662,6 +1665,9 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  			continue;
>  		}
>  
> +		/* Ask next iteration to move us */
> +		need_move = 1;
> +
>  		/*
>  		 * Two possible cases for err here:
>  		 * == 0: page is already on the target node, then store
> @@ -1671,17 +1677,11 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  		err = store_status(status, i, err ? : current_node, 1);
>  		if (err)
>  			goto out_flush;
> -
> -		err = move_pages_and_store_status(mm, current_node, &pagelist,
> -				status, start, i - start);
> -		if (err)
> -			goto out;
> -		current_node = NUMA_NO_NODE;
>  	}
>  out_flush:
>  	/* Make sure we do not overwrite the existing error */
>  	err1 = move_pages_and_store_status(mm, current_node, &pagelist,
> -				status, start, i - start);
> +				status, start, i - start - need_move);
>  	if (err >= 0)
>  		err = err1;
>  out:
> -- 
> 2.17.1
> 

-- 
Michal Hocko
SUSE Labs