On Wed, Apr 18, 2018 at 5:19 PM, Michal Hocko <mhocko@xxxxxxxx> 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@xxxxxxxxxx
liwang@xxxxxxxxxx
