Re: [PATCH] mm: Always downgrade mmap_lock if requested

[Linus Torvalds]

On Thu, 29 Jun 2023 at 12:14, Matthew Wilcox (Oracle)
<willy@xxxxxxxxxxxxx> wrote:
>
> Now that stack growth must always hold the mmap_lock for write, we can
> always downgrade the mmap_lock to read and safely unmap pages from the
> page table, even if we're next to a stack.

Can we please also fix the really odd return value semantics?

Right now that function returns either an error - meaning that it
didn't downgrade, or it returns 0/1 as a success to show "did I
downgrade as you asked me to"?

That is *really* confusing, but it was needed in that bad old world order.

But now that the downgrade is not a "try to downgrade if you can", but
something reliable, can we please just make the success case be 0, and
make the callers all know that on success, it was downgraded?

And yes, I realize that that means do_vmi_munmap() also has to be
changed. The documentation for that function is horrid, btw, in that
it says

 * Returns: -EINVAL on failure, 1 on success and unlock, 0 otherwise.

which is just not true. It can return other errors than -EINVAL
(through exactly that do_vmi_align_munmap() function), and the "1 on
success and unlock" is not true, it's a "success and downgrade if you
asked me to".

So I think all of those callers should also be changed to "if you
asked for a downgrade, and do_vmi_munmap() returned success, then you
got a downgrade".

Then some of the callers of *that* can be simplified too.

Please?

                 Linus