On Wed 13-12-17 15:47:31, Kirill A. Shutemov wrote:
> On Wed, Dec 13, 2017 at 01:17:03PM +0100, Michal Hocko wrote:
> > On Wed 13-12-17 15:07:33, Kirill A. Shutemov wrote:
> > [...]
> > > The approach looks fine to me.
> > >
> > > But patch is rather large and hard to review. And how git mixed add/remove
> > > lines doesn't help too. Any chance to split it up further?
> >
> > I was trying to do that but this is a drop in replacement so it is quite
> > hard to do in smaller pieces. I've already put the allocation callback
> > cleanup into a separate one but this is about all that I figured how to
> > split. If you have any suggestions I am willing to try them out.
>
> "git diff --patience" seems generate more readable output for the patch.
Hmm, I wasn't aware of this option. Are you suggesting I should use it
to general the patch to send?
> > > One nitpick: I don't think 'chunk' terminology should go away with the
> > > patch.
> >
> > Not sure what you mean here. I have kept chunk_start, chunk_node, so I
> > am not really changing that terminology
>
> We don't really have chunks anymore, right? We still *may* have per-node
> batching, but..
>
> Maybe just 'start' and 'current_node'?
Ohh, I've read your response that you want to preserve the naming. I can
certainly do the rename.
---
diff --git a/mm/migrate.c b/mm/migrate.c
index 9d7252ea2acd..5491045b60f9 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1556,14 +1556,14 @@ static int do_pages_move(struct mm_struct *mm,
nodemask_t task_nodes,
const int __user *nodes,
int __user *status, int flags)
{
- int chunk_node = NUMA_NO_NODE;
+ int current_node = NUMA_NO_NODE;
LIST_HEAD(pagelist);
- int chunk_start, i;
+ int start, i;
int err = 0, err1;
migrate_prep();
- for (i = chunk_start = 0; i < nr_pages; i++) {
+ for (i = start = 0; i < nr_pages; i++) {
const void __user *p;
unsigned long addr;
int node;
@@ -1585,25 +1585,25 @@ static int do_pages_move(struct mm_struct *mm,
nodemask_t task_nodes,
if (!node_isset(node, task_nodes))
goto out_flush;
- if (chunk_node == NUMA_NO_NODE) {
- chunk_node = node;
- chunk_start = i;
- } else if (node != chunk_node) {
- err = do_move_pages_to_node(mm, &pagelist, chunk_node);
+ if (current_node == NUMA_NO_NODE) {
+ 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, chunk_start, chunk_node, i -
chunk_start);
+ err = store_status(status, start, current_node, i -
start);
if (err)
goto out;
- chunk_start = i;
- chunk_node = node;
+ start = i;
+ current_node = node;
}
/*
* Errors in the page lookup or isolation are not fatal and we
simply
* report them via status
*/
- err = add_page_for_migration(mm, addr, chunk_node,
+ err = add_page_for_migration(mm, addr, current_node,
&pagelist, flags & MPOL_MF_MOVE_ALL);
if (!err)
continue;
@@ -1612,22 +1612,22 @@ 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, chunk_node);
+ err = do_move_pages_to_node(mm, &pagelist, current_node);
if (err)
goto out;
- if (i > chunk_start) {
- err = store_status(status, chunk_start, chunk_node, i -
chunk_start);
+ if (i > start) {
+ err = store_status(status, start, current_node, i -
start);
if (err)
goto out;
}
- chunk_node = NUMA_NO_NODE;
+ current_node = NUMA_NO_NODE;
}
err = 0;
out_flush:
/* Make sure we do not overwrite the existing error */
- err1 = do_move_pages_to_node(mm, &pagelist, chunk_node);
+ err1 = do_move_pages_to_node(mm, &pagelist, current_node);
if (!err1)
- err1 = store_status(status, chunk_start, chunk_node, i -
chunk_start);
+ err1 = store_status(status, start, current_node, i - start);
if (!err)
err = err1;
out:
--
Michal Hocko
SUSE Labs
