Re: [PATCH v3 2/6] x86/uaccess: Avoid barrier_nospec() in 64-bit __get_user()

From: Josh Poimboeuf
Date: Thu Nov 21 2024 - 16:40:26 EST


> On Fri, 15 Nov 2024 at 15:06, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> So I think the thing to do is
>
> (a) find out which __get_user() it is that matters so much for that load
>
> Do you have a profile somewhere?
>
> (b) convert them to use "unsafe_get_user()", with that whole
>
> if (can_do_masked_user_access())
> from = masked_user_access_begin(from);
> else if (!user_read_access_begin(from, sizeof(*from)))
> return -EFAULT;
>
> sequence before it.
>
> And if it's just a single __get_user() (rather than a sequence of
> them), just convert it to get_user().
>
> Hmm?

The profile is showing futex_get_value_locked():

int futex_get_value_locked(u32 *dest, u32 __user *from)
{
int ret;

pagefault_disable();
ret = __get_user(*dest, from);
pagefault_enable();

return ret ? -EFAULT : 0;
}

That has several callers, so we can probably just use get_user() there?

Also, is there any harm in speeding up __get_user()? It still has ~80
callers and it's likely to be slowing down things we don't know about.

It's usually only the regressions which get noticed, and that LFENCE
went in almost 7 years ago, when there was much less automated
performance regression testing.

As a bonus, that patch will root out any "bad" users, which will
eventually allow us to simplify things and just make __get_user() an
alias of get_user().

In fact, if we aliased it for all arches, that could help in getting rid
of __get_user() altogether as there would no longer be any (real or
advertised) benefit to using it.

--
Josh