On Wed 18-04-18 18:39:19, Li Wang wrote: > 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. Thanks for retesting! Do you plan to post a patch with the changelog or should I do it? -- Michal Hocko SUSE Labs
