Date:   Wed, 9 Sep 2020 10:31:34 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Christoph Hellwig <hch@....de>,
        Michael Ellerman <mpe@...erman.id.au>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: remove the last set_fs() in common code, and remove it for x86
 and powerpc v3

On Thu, Sep 3, 2020 at 7:28 AM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> I can live with this series; do you want that in vfs.git#for-next?

Well, it's apparently there now (at least it's in your base.set_fs
branch, I didn't check actual -next).

So this is just a heads-up that I plan to merge the "asm goto" changes
on top of this during 5.10. Nick did the patch to make my patch-set
work either with or without the actual asm goto support, and I've been
running it privately now for several months.

And apparently there are people working on this on the gcc side too,
so it won't just be clang-specific. Nor kernel-specific in that Nick
tells me some other projects are looking at using that asm goto with
outputs too.

Anyway, the actual patch to use asm goto with outputs is fairly small
and not that interesting to people (since no released compiler
supports it), but part of the infrastructure to make it tiny is to
just get rid of the inlined "__get_user()" and "__put_user()" stuff.
I've ranted against those functions for a few years by now, so part of
this is to stop inlining them and make people think they are "good",
but part of it is also that those macros and inline functions are the
main remaining ones that mess with this all.

I'm attaching the two __get_user/__put_user patches here in case
anybody cares, but these are the pre-rebased ones, I'll make them work
with the new world order as it happens. The main change is:

 (a) unify around a common special calling convention:
   - %bx is clobbered
   - %cx contains the user address on input, and the error value on output
   - %ax/%dx contains the actual value (input for put, output for get,
of course)

 (b) unify around using just a "call", using the model that
get/put_user already did.
   - use "*_nocheck" for the double-underscore versions
   - this still has to use inline asm because the calling convention is odd
   - otherwise basically just a "call __{get,put}_user_[nocheck_]X"
where X is the size.

IOW, we unify around one single calling convention., and one single
model for actually getting things done.

I still want to remove the double-underscore versions entirely some
day - they have absolutely zero advantages compared to the full "do
address_ok as part of the operation" - but that's a separate thing. At
least they can be unified.

And the reason for this all is obviously that now *only* the
"unsafe_{get,put}_user()" cases with the error label output are the
"fast inlined" cases. They are the only ones that _can_ be done
quickly inline, since the slow clac/stac is not part of them. Plus
they already have that unified usage model of the error label, even if
unsafe_get_user() currently does it manually because "asm goto" with
outputs doesn't work in existing compilers.

Comments?

I suspect people won't care, but I thought I'd post these so that
there won't be any surprises during the next merge window when I apply
them after merging the set_fs() removal branch..

                 Linus

View attachment "0001-x86-Make-__get_user-generate-an-out-of-line-call.patch" of type "text/x-patch" (10283 bytes)

View attachment "0002-x86-Make-__put_user-generate-an-out-of-line-call.patch" of type "text/x-patch" (9905 bytes)

Powered by blists - more mailing lists