On 22.01.20 02:16, 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. I'd suggest " Usually, do_move_pages_to_node() and store_status() are used in combination. We have three similar call sites. Let's provide a wrapper for both function calls - move_pages_and_store_status - to make the calling code easier to maintain and fix (as noted by Yang Shi, the return value handling of do_move_pages_to_node() has a flaw). " > > 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@xxxxxxxxxxxxxxx> > Acked-by: Michal Hocko <mhocko@xxxxxxxx> > --- > mm/migrate.c | 30 +++++++++++++++++++----------- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 4c2a21856717..a4d3bd6475e1 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) nit: indentation > +{ > + int err; > + > + err = do_move_pages_to_node(mm, pagelist, node); > + if (err) > + return err; > + err = store_status(status, start, node, nr); > + return err; return store_status(status, start, node, nr); directly Apart from that (and some more indentation nits) Reviewed-by: David Hildenbrand <david@xxxxxxxxxx> -- Thanks, David / dhildenb