From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua0-f200.google.com (mail-ua0-f200.google.com [209.85.217.200]) by kanga.kvack.org (Postfix) with ESMTP id B6A686B0005 for ; Wed, 18 Apr 2018 07:46:28 -0400 (EDT) Received: by mail-ua0-f200.google.com with SMTP id v5so969749uae.10 for ; Wed, 18 Apr 2018 04:46:28 -0700 (PDT) Received: from mail-sor-f41.google.com (mail-sor-f41.google.com. [209.85.220.41]) by mx.google.com with SMTPS id i12sor376962uaj.206.2018.04.18.04.46.27 for (Google Transport Security); Wed, 18 Apr 2018 04:46:27 -0700 (PDT) MIME-Version: 1.0 References: <20180417110615.16043-1-liwang@redhat.com> <20180417130300.GF17484@dhcp22.suse.cz> <20180417141442.GG17484@dhcp22.suse.cz> <20180417190044.GK17484@dhcp22.suse.cz> <7674C632-FE3E-42D2-B19D-32F531617043@cs.rutgers.edu> <20180418090722.GV17484@dhcp22.suse.cz> <20180418091943.GW17484@dhcp22.suse.cz> <20180418112916.GX17484@dhcp22.suse.cz> In-Reply-To: <20180418112916.GX17484@dhcp22.suse.cz> From: Li Wang Date: Wed, 18 Apr 2018 11:46:17 +0000 Message-ID: Subject: Re: [RFC PATCH] mm: correct status code which move_pages() returns for zero page Content-Type: multipart/alternative; boundary="089e08240fd057ed1d056a1e01bc" Sender: owner-linux-mm@kvack.org List-ID: To: Michal Hocko Cc: Zi Yan , linux-mm@kvack.org, ltp@lists.linux.it, "Kirill A . Shutemov" --089e08240fd057ed1d056a1e01bc Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Apr 18, 2018, 19:29 Michal Hocko wrote: > On Wed 18-04-18 18:39:19, Li Wang wrote: > > On Wed, Apr 18, 2018 at 5:19 PM, Michal Hocko 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 =3D i + 1; > > > > > current_node =3D 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 co= de > > > > 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 fo= r > > > 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. > > > > > > > =E2=80=8BHi 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? > You better. Li Wang --089e08240fd057ed1d056a1e01bc Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


= On Wed, Apr 18, 2018, 19:29 Michal Hocko <mhocko@suse.com> wrote:
On Wed 18-04-18 18:39:19, Li Wang wro= te:
> 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 m= m_struct *mm,
> > nodemask_t task_nodes,
> > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (err)
> > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto out;
> > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0}
> > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0/* Move to next page (i+1), after we have saved page
> > status (until i) */
> > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0start =3D i + 1;
> > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0current_node =3D NUMA_NO_NODE;
> > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> > > >=C2=A0 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 f= ail 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 chec= k for
> > list_empty on the page list. It is a bit larger diff but maybe th= at
> > would be tiny bit cleaner because there is simply no point to cal= l
> > do_move_pages_to_node on an empty list in the first place.
> >
>
> =E2=80=8BHi 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?

You better.

Li Wang
--089e08240fd057ed1d056a1e01bc--