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

[Josh Poimboeuf]

> 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