| Message ID | 20200119030636.11899-4-richardw.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/migrate.c: cleanup on do_pages_move() | expand |
On Sun 19-01-20 11:06:31, Wei Yang wrote: > No functional change, just reform it to make it as the same shape as > other calls on do_move_pages_to_node(). > > This is a preparation for further cleanup. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > --- > mm/migrate.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index c3ef70de5876..4a63fb8fbb6d 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1675,8 +1675,12 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes, > > /* Make sure we do not overwrite the existing error */ > err1 = do_move_pages_to_node(mm, &pagelist, current_node); > - if (!err1) > - err1 = store_status(status, start, current_node, i - start); > + if (err1) { > + if (err >= 0) > + err = err1; > + goto out; > + } > + err1 = store_status(status, start, current_node, i - start); Please don't. This just makes the code harder to follow. The current err and err1 is already quite ugly so do not make it more so. > if (err >= 0) > err = err1; > out: > -- > 2.17.1
On Mon, Jan 20, 2020 at 10:46:08AM +0100, Michal Hocko wrote: >On Sun 19-01-20 11:06:31, Wei Yang wrote: >> No functional change, just reform it to make it as the same shape as >> other calls on do_move_pages_to_node(). >> >> This is a preparation for further cleanup. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> --- >> mm/migrate.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index c3ef70de5876..4a63fb8fbb6d 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1675,8 +1675,12 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes, >> >> /* Make sure we do not overwrite the existing error */ >> err1 = do_move_pages_to_node(mm, &pagelist, current_node); >> - if (!err1) >> - err1 = store_status(status, start, current_node, i - start); >> + if (err1) { >> + if (err >= 0) >> + err = err1; >> + goto out; >> + } >> + err1 = store_status(status, start, current_node, i - start); > >Please don't. This just makes the code harder to follow. The current err >and err1 is already quite ugly so do not make it more so. > Yes, I struggled a little on doing this change. Sounds we can merge this one with the following consolidation. >> if (err >= 0) >> err = err1; >> out: >> -- >> 2.17.1 > >-- >Michal Hocko >SUSE Labs
diff --git a/mm/migrate.c b/mm/migrate.c index c3ef70de5876..4a63fb8fbb6d 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1675,8 +1675,12 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes, /* Make sure we do not overwrite the existing error */ err1 = do_move_pages_to_node(mm, &pagelist, current_node); - if (!err1) - err1 = store_status(status, start, current_node, i - start); + if (err1) { + if (err >= 0) + err = err1; + goto out; + } + err1 = store_status(status, start, current_node, i - start); if (err >= 0) err = err1; out:
No functional change, just reform it to make it as the same shape as other calls on do_move_pages_to_node(). This is a preparation for further cleanup. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- mm/migrate.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)