Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230518021825.712742-1-joel@joelfernandes.org> <20230518021825.712742-2-joel@joelfernandes.org>
In-Reply-To: <20230518021825.712742-2-joel@joelfernandes.org>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 17 May 2023 21:12:07 -0700
Message-ID: <CAHk-=wi20GzWeK=vUoreX6AXOEmeJKczou1McCOC9JytVO=TQg@mail.gmail.com>
Subject: Re: [PATCH RFC 1/4] mm/mremap: Optimize the start addresses in move_page_tables()
To:     "Joel Fernandes (Google)" <joel@joelfernandes.org>
Cc:     linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-mm@kvack.org, Shuah Khan <shuah@kernel.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Michal Hocko <mhocko@suse.com>,
        Lorenzo Stoakes <lstoakes@gmail.com>,
        Kirill A Shutemov <kirill@shutemov.name>,
        "Liam R. Howlett" <liam.howlett@oracle.com>,
        "Paul E. McKenney" <paulmck@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, May 17, 2023 at 7:18=E2=80=AFPM Joel Fernandes (Google)
<joel@joelfernandes.org> wrote:
>
> This warning will only trigger when there is mutual alignment in the
> move operation. A solution, as suggested by Linus Torvalds [2], is to
> initiate the copy process at the PMD level whenever such alignment is
> present.

So this patch is actually simpler than I thought it would be.

But I'm a bit nervous about it. In particular, it ends doing

        old_end =3D old_addr + len;
        ... expand old_addr/new_addr down to the pmd boundary ..
        return len + old_addr - old_end;        /* how much done */

doesn't that return value end up being nonsensical now?

In particular, I think it can return a *negative* value, because of
how old_addr was moved down, and the "now much done" might indeed be
"negative" in the sense that it failed the move even "before" the
original starting point.

And that negative value then ends up being a large positive one as an
"unsigned long", of course.

So I get the feeling that it wants something like

        if (old_addr + len < old_end)
                return 0;

there at the end.

But maybe there is something in there that guarantees that that case
never happens. I didn't think too deeply about it, I just felt this
looked odd.

               Linus