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

[Li Wang <liwang@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

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.
>

​Hi Michal, Zi

I tried your patch separately, both of them works fine to me.


-- 
Li Wang
liwang@redhat.com

[-- Attachment #2: Type: text/html, Size: 3012 bytes --]