[4/8] mm/migrate.c: wrap do_move_pages_to_node() and store_status()
diff mbox series

Message ID 20200119030636.11899-5-richardw.yang@linux.intel.com
State New
Headers show
Series
  • mm/migrate.c: cleanup on do_pages_move()
Related show

Commit Message

Wei Yang Jan. 19, 2020, 3:06 a.m. UTC
Usually do_move_pages_to_node() and store_status() is a pair. There are
three places call this pair of functions with almost the same form.

This patch just wrap it to make it friendly to audience and also
consolidate the move and store action into one place. Also mentioned by
Yang Shi, the handling of do_move_pages_to_node()'s return value is not
proper. Now we can fix it in one place.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
 mm/migrate.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

Comments

Michal Hocko Jan. 20, 2020, 9:49 a.m. UTC | #1
On Sun 19-01-20 11:06:32, Wei Yang wrote:
> Usually do_move_pages_to_node() and store_status() is a pair. There are
> three places call this pair of functions with almost the same form.
> 
> This patch just wrap it to make it friendly to audience and also
> consolidate the move and store action into one place. Also mentioned by
> Yang Shi, the handling of do_move_pages_to_node()'s return value is not
> proper. Now we can fix it in one place.

The helper helps here indeed. Thanks this makes the code easier to read.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

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

> ---
>  mm/migrate.c | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 4a63fb8fbb6d..dec147d3a4dd 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1583,6 +1583,19 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
>  	return err;
>  }
>  
> +static int move_pages_and_store_status(struct mm_struct *mm, int node,
> +		struct list_head *pagelist, int __user *status,
> +		int start, int nr)
> +{
> +	int err;
> +
> +	err = do_move_pages_to_node(mm, pagelist, node);
> +	if (err)
> +		return err;
> +	err = store_status(status, start, node, nr);
> +	return err;
> +}
> +
>  /*
>   * Migrate an array of page address onto an array of nodes and fill
>   * the corresponding array of status.
> @@ -1629,10 +1642,8 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  			current_node = node;
>  			start = i;
>  		} else if (node != current_node) {
> -			err = do_move_pages_to_node(mm, &pagelist, current_node);
> -			if (err)
> -				goto out;
> -			err = store_status(status, start, current_node, i - start);
> +			err = move_pages_and_store_status(mm, current_node,
> +					&pagelist, status, start, i - start);
>  			if (err)
>  				goto out;
>  			start = i;
> @@ -1661,10 +1672,8 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  		if (err)
>  			goto out_flush;
>  
> -		err = do_move_pages_to_node(mm, &pagelist, current_node);
> -		if (err)
> -			goto out;
> -		err = store_status(status, start, current_node, i - start);
> +		err = move_pages_and_store_status(mm, current_node, &pagelist,
> +				status, start, i - start);
>  		if (err)
>  			goto out;
>  		current_node = NUMA_NO_NODE;
> @@ -1674,13 +1683,8 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
>  		return err;
>  
>  	/* Make sure we do not overwrite the existing error */
> -	err1 = do_move_pages_to_node(mm, &pagelist, current_node);
> -	if (err1) {
> -		if (err >= 0)
> -			err = err1;
> -		goto out;
> -	}
> -	err1 = store_status(status, start, current_node, i - start);
> +	err1 = move_pages_and_store_status(mm, current_node, &pagelist,
> +				status, start, i - start);
>  	if (err >= 0)
>  		err = err1;
>  out:
> -- 
> 2.17.1
>

Patch
diff mbox series

diff --git a/mm/migrate.c b/mm/migrate.c
index 4a63fb8fbb6d..dec147d3a4dd 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1583,6 +1583,19 @@  static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
 	return err;
 }
 
+static int move_pages_and_store_status(struct mm_struct *mm, int node,
+		struct list_head *pagelist, int __user *status,
+		int start, int nr)
+{
+	int err;
+
+	err = do_move_pages_to_node(mm, pagelist, node);
+	if (err)
+		return err;
+	err = store_status(status, start, node, nr);
+	return err;
+}
+
 /*
  * Migrate an array of page address onto an array of nodes and fill
  * the corresponding array of status.
@@ -1629,10 +1642,8 @@  static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
 			current_node = node;
 			start = i;
 		} else if (node != current_node) {
-			err = do_move_pages_to_node(mm, &pagelist, current_node);
-			if (err)
-				goto out;
-			err = store_status(status, start, current_node, i - start);
+			err = move_pages_and_store_status(mm, current_node,
+					&pagelist, status, start, i - start);
 			if (err)
 				goto out;
 			start = i;
@@ -1661,10 +1672,8 @@  static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
 		if (err)
 			goto out_flush;
 
-		err = do_move_pages_to_node(mm, &pagelist, current_node);
-		if (err)
-			goto out;
-		err = store_status(status, start, current_node, i - start);
+		err = move_pages_and_store_status(mm, current_node, &pagelist,
+				status, start, i - start);
 		if (err)
 			goto out;
 		current_node = NUMA_NO_NODE;
@@ -1674,13 +1683,8 @@  static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
 		return err;
 
 	/* Make sure we do not overwrite the existing error */
-	err1 = do_move_pages_to_node(mm, &pagelist, current_node);
-	if (err1) {
-		if (err >= 0)
-			err = err1;
-		goto out;
-	}
-	err1 = store_status(status, start, current_node, i - start);
+	err1 = move_pages_and_store_status(mm, current_node, &pagelist,
+				status, start, i - start);
 	if (err >= 0)
 		err = err1;
 out: