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.
Signed-off-by: Wei Yang <[email protected]>
---
mm/migrate.c | 61 ++++++++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index ae3db45c6a42..e816c8990296 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1583,6 +1583,29 @@ 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,
+ unsigned long nr_pages, int start, int i)
+{
+ int err;
+
+ err = do_move_pages_to_node(mm, pagelist, node);
+ if (err) {
+ /*
+ * Positive err means the number of failed
+ * pages to migrate. Since we are going to
+ * abort and return the number of non-migrated
+ * pages, so need incude the rest of the
+ * nr_pages that have not attempted as well.
+ */
+ if (err > 0)
+ err += nr_pages - i - 1;
+ return err;
+ }
+ err = store_status(status, start, node, i - start);
+ return err;
+}
+
/*
* Migrate an array of page address onto an array of nodes and fill
* the corresponding array of status.
@@ -1626,20 +1649,9 @@ 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) {
- /*
- * Positive err means the number of failed
- * pages to migrate. Since we are going to
- * abort and return the number of non-migrated
- * pages, so need incude the rest of the
- * nr_pages that have not attempted as well.
- */
- if (err > 0)
- err += nr_pages - i - 1;
- goto out;
- }
- err = store_status(status, start, current_node, i - start);
+ err = move_pages_and_store_status(mm, current_node,
+ &pagelist, status, nr_pages,
+ start, i);
if (err)
goto out;
start = i;
@@ -1668,13 +1680,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) {
- if (err > 0)
- err += nr_pages - i - 1;
- goto out;
- }
- err = store_status(status, start, current_node, i - start);
+ err = move_pages_and_store_status(mm, current_node, &pagelist,
+ status, nr_pages, start, i);
if (err)
goto out;
current_node = NUMA_NO_NODE;
@@ -1684,16 +1691,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);
- /*
- * Don't have to report non-attempted pages here since:
- * - If the above loop is done gracefully there is not non-attempted
- * page.
- * - If the above loop is aborted to it means more fatal error
- * happened, should return err.
- */
- if (!err1)
- err1 = store_status(status, start, current_node, i - start);
+ err1 = move_pages_and_store_status(mm, current_node, &pagelist,
+ status, nr_pages, start, i);
if (err >= 0)
err = err1;
out:
--
2.17.1
On Sun 26-01-20 18:26:21, 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.
>
> Signed-off-by: Wei Yang <[email protected]>
I believe I have already acked this one. Anyway
Acked-by: Michal Hocko <[email protected]>
> ---
> mm/migrate.c | 61 ++++++++++++++++++++++++++--------------------------
> 1 file changed, 30 insertions(+), 31 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index ae3db45c6a42..e816c8990296 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1583,6 +1583,29 @@ 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,
> + unsigned long nr_pages, int start, int i)
> +{
> + int err;
> +
> + err = do_move_pages_to_node(mm, pagelist, node);
> + if (err) {
> + /*
> + * Positive err means the number of failed
> + * pages to migrate. Since we are going to
> + * abort and return the number of non-migrated
> + * pages, so need incude the rest of the
> + * nr_pages that have not attempted as well.
> + */
> + if (err > 0)
> + err += nr_pages - i - 1;
> + return err;
> + }
> + err = store_status(status, start, node, i - start);
> + return err;
> +}
> +
> /*
> * Migrate an array of page address onto an array of nodes and fill
> * the corresponding array of status.
> @@ -1626,20 +1649,9 @@ 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) {
> - /*
> - * Positive err means the number of failed
> - * pages to migrate. Since we are going to
> - * abort and return the number of non-migrated
> - * pages, so need incude the rest of the
> - * nr_pages that have not attempted as well.
> - */
> - if (err > 0)
> - err += nr_pages - i - 1;
> - goto out;
> - }
> - err = store_status(status, start, current_node, i - start);
> + err = move_pages_and_store_status(mm, current_node,
> + &pagelist, status, nr_pages,
> + start, i);
> if (err)
> goto out;
> start = i;
> @@ -1668,13 +1680,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) {
> - if (err > 0)
> - err += nr_pages - i - 1;
> - goto out;
> - }
> - err = store_status(status, start, current_node, i - start);
> + err = move_pages_and_store_status(mm, current_node, &pagelist,
> + status, nr_pages, start, i);
> if (err)
> goto out;
> current_node = NUMA_NO_NODE;
> @@ -1684,16 +1691,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);
> - /*
> - * Don't have to report non-attempted pages here since:
> - * - If the above loop is done gracefully there is not non-attempted
> - * page.
> - * - If the above loop is aborted to it means more fatal error
> - * happened, should return err.
> - */
> - if (!err1)
> - err1 = store_status(status, start, current_node, i - start);
> + err1 = move_pages_and_store_status(mm, current_node, &pagelist,
> + status, nr_pages, start, i);
> if (err >= 0)
> err = err1;
> out:
> --
> 2.17.1
--
Michal Hocko
SUSE Labs
On Mon, Jan 27, 2020 at 10:36:42AM +0100, Michal Hocko wrote:
>On Sun 26-01-20 18:26:21, 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.
>>
>> Signed-off-by: Wei Yang <[email protected]>
>
>I believe I have already acked this one. Anyway
>Acked-by: Michal Hocko <[email protected]>
>
Yep, since some change from original code, I dropped your ack.
Thanks
>> ---
>> mm/migrate.c | 61 ++++++++++++++++++++++++++--------------------------
>> 1 file changed, 30 insertions(+), 31 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index ae3db45c6a42..e816c8990296 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -1583,6 +1583,29 @@ 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,
>> + unsigned long nr_pages, int start, int i)
>> +{
>> + int err;
>> +
>> + err = do_move_pages_to_node(mm, pagelist, node);
>> + if (err) {
>> + /*
>> + * Positive err means the number of failed
>> + * pages to migrate. Since we are going to
>> + * abort and return the number of non-migrated
>> + * pages, so need incude the rest of the
>> + * nr_pages that have not attempted as well.
>> + */
>> + if (err > 0)
>> + err += nr_pages - i - 1;
>> + return err;
>> + }
>> + err = store_status(status, start, node, i - start);
>> + return err;
>> +}
>> +
>> /*
>> * Migrate an array of page address onto an array of nodes and fill
>> * the corresponding array of status.
>> @@ -1626,20 +1649,9 @@ 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) {
>> - /*
>> - * Positive err means the number of failed
>> - * pages to migrate. Since we are going to
>> - * abort and return the number of non-migrated
>> - * pages, so need incude the rest of the
>> - * nr_pages that have not attempted as well.
>> - */
>> - if (err > 0)
>> - err += nr_pages - i - 1;
>> - goto out;
>> - }
>> - err = store_status(status, start, current_node, i - start);
>> + err = move_pages_and_store_status(mm, current_node,
>> + &pagelist, status, nr_pages,
>> + start, i);
>> if (err)
>> goto out;
>> start = i;
>> @@ -1668,13 +1680,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) {
>> - if (err > 0)
>> - err += nr_pages - i - 1;
>> - goto out;
>> - }
>> - err = store_status(status, start, current_node, i - start);
>> + err = move_pages_and_store_status(mm, current_node, &pagelist,
>> + status, nr_pages, start, i);
>> if (err)
>> goto out;
>> current_node = NUMA_NO_NODE;
>> @@ -1684,16 +1691,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);
>> - /*
>> - * Don't have to report non-attempted pages here since:
>> - * - If the above loop is done gracefully there is not non-attempted
>> - * page.
>> - * - If the above loop is aborted to it means more fatal error
>> - * happened, should return err.
>> - */
>> - if (!err1)
>> - err1 = store_status(status, start, current_node, i - start);
>> + err1 = move_pages_and_store_status(mm, current_node, &pagelist,
>> + status, nr_pages, start, i);
>> if (err >= 0)
>> err = err1;
>> out:
>> --
>> 2.17.1
>
>--
>Michal Hocko
>SUSE Labs
--
Wei Yang
Help you, Help me