Re: [RFC PATCH] mm: correct status code which move_pages() returns for zero page

[Michal Hocko <mhocko@suse.com>]

On Wed 18-04-18 18:39:19, Li Wang wrote:
> On Wed, Apr 18, 2018 at 5:19 PM, Michal Hocko <mhocko@suse.com> wrote:
> 
> > On Wed 18-04-18 11:07:22, Michal Hocko wrote:
> > > On Tue 17-04-18 16:09:33, Zi Yan wrote:
> > [...]
> > > > diff --git a/mm/migrate.c b/mm/migrate.c
> > > > index f65dd69e1fd1..32afa4723e7f 100644
> > > > --- a/mm/migrate.c
> > > > +++ b/mm/migrate.c
> > > > @@ -1619,6 +1619,8 @@ static int do_pages_move(struct mm_struct *mm,
> > nodemask_t task_nodes,
> > > >                         if (err)
> > > >                                 goto out;
> > > >                 }
> > > > +               /* Move to next page (i+1), after we have saved page
> > status (until i) */
> > > > +               start = i + 1;
> > > >                 current_node = NUMA_NO_NODE;
> > > >         }
> > > >  out_flush:
> > > >
> > > > Feel free to check it by yourselves.
> > >
> > > Yes, you are right. I never update start if the last page in the range
> > > fails and so we overwrite the whole [start, i] range. I wish the code
> > > wasn't that ugly and subtle but considering how we can fail in different
> > > ways and that we want to batch as much as possible I do not see an easy
> > > way.
> > >
> > > Care to send the patch? I would just drop the comment.
> >
> > Hmm, thinking about it some more. An alternative would be to check for
> > list_empty on the page list. It is a bit larger diff but maybe that
> > would be tiny bit cleaner because there is simply no point to call
> > do_move_pages_to_node on an empty list in the first place.
> >
> 
> a??Hi Michal, Zi
> 
> I tried your patch separately, both of them works fine to me.

Thanks for retesting! Do you plan to post a patch with the changelog or
should I do it?
-- 
Michal Hocko
SUSE Labs