netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
@ 2025-11-13  0:55 Jon Kohler
  2025-11-13  1:09 ` Jason Wang
  2025-11-14 18:54 ` David Laight
  0 siblings, 2 replies; 27+ messages in thread
From: Jon Kohler @ 2025-11-13  0:55 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, Eugenio Pérez, kvm,
	virtualization, netdev, linux-kernel
  Cc: Jon Kohler, Linus Torvalds, Borislav Petkov, Sean Christopherson

vhost_get_user and vhost_put_user leverage __get_user and __put_user,
respectively, which were both added in 2016 by commit 6b1e6cc7855b
("vhost: new device IOTLB API"). In a heavy UDP transmit workload on a
vhost-net backed tap device, these functions showed up as ~11.6% of
samples in a flamegraph of the underlying vhost worker thread.

Quoting Linus from [1]:
    Anyway, every single __get_user() call I looked at looked like
    historical garbage. [...] End result: I get the feeling that we
    should just do a global search-and-replace of the __get_user/
    __put_user users, replace them with plain get_user/put_user instead,
    and then fix up any fallout (eg the coco code).

Switch to plain get_user/put_user in vhost, which results in a slight
throughput speedup. get_user now about ~8.4% of samples in flamegraph.

Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
RX: taskset -c 2 iperf3 -s -p 5200 -D
Before: 6.08 Gbits/sec
After:  6.32 Gbits/sec

As to what drives the speedup, Sean's patch [2] explains:
	Use the normal, checked versions for get_user() and put_user() instead of
	the double-underscore versions that omit range checks, as the checked
	versions are actually measurably faster on modern CPUs (12%+ on Intel,
	25%+ on AMD).

	The performance hit on the unchecked versions is almost entirely due to
	the added LFENCE on CPUs where LFENCE is serializing (which is effectively
	all modern CPUs), which was added by commit 304ec1b05031 ("x86/uaccess:
	Use __uaccess_begin_nospec() and uaccess_try_nospec").  The small
	optimizations done by commit b19b74bc99b1 ("x86/mm: Rework address range
	check in get_user() and put_user()") likely shave a few cycles off, but
	the bulk of the extra latency comes from the LFENCE.

[1] https://lore.kernel.org/all/CAHk-=wiJiDSPZJTV7z3Q-u4DfLgQTNWqUqqrwSBHp0+Dh016FA@mail.gmail.com/
[2] https://lore.kernel.org/all/20251106210206.221558-1-seanjc@google.com/

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Sean Christopherson <seanjc@google.com>
Signed-off-by: Jon Kohler <jon@nutanix.com>
---
 drivers/vhost/vhost.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 8570fdf2e14a..ffbd0a9a7a03 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1442,13 +1442,13 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
 ({ \
 	int ret; \
 	if (!vq->iotlb) { \
-		ret = __put_user(x, ptr); \
+		ret = put_user(x, ptr); \
 	} else { \
 		__typeof__(ptr) to = \
 			(__typeof__(ptr)) __vhost_get_user(vq, ptr,	\
 					  sizeof(*ptr), VHOST_ADDR_USED); \
 		if (to != NULL) \
-			ret = __put_user(x, to); \
+			ret = put_user(x, to); \
 		else \
 			ret = -EFAULT;	\
 	} \
@@ -1487,14 +1487,14 @@ static inline int vhost_put_used_idx(struct vhost_virtqueue *vq)
 ({ \
 	int ret; \
 	if (!vq->iotlb) { \
-		ret = __get_user(x, ptr); \
+		ret = get_user(x, ptr); \
 	} else { \
 		__typeof__(ptr) from = \
 			(__typeof__(ptr)) __vhost_get_user(vq, ptr, \
 							   sizeof(*ptr), \
 							   type); \
 		if (from != NULL) \
-			ret = __get_user(x, from); \
+			ret = get_user(x, from); \
 		else \
 			ret = -EFAULT; \
 	} \
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-13  0:55 [PATCH net-next] vhost: use "checked" versions of get_user() and put_user() Jon Kohler
@ 2025-11-13  1:09 ` Jason Wang
  2025-11-14 14:53   ` Jon Kohler
  2025-11-14 18:54 ` David Laight
  1 sibling, 1 reply; 27+ messages in thread
From: Jason Wang @ 2025-11-13  1:09 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm, virtualization,
	netdev, linux-kernel, Linus Torvalds, Borislav Petkov,
	Sean Christopherson

On Thu, Nov 13, 2025 at 8:14 AM Jon Kohler <jon@nutanix.com> wrote:
>
> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
> respectively, which were both added in 2016 by commit 6b1e6cc7855b
> ("vhost: new device IOTLB API").

It has been used even before this commit.

> In a heavy UDP transmit workload on a
> vhost-net backed tap device, these functions showed up as ~11.6% of
> samples in a flamegraph of the underlying vhost worker thread.
>
> Quoting Linus from [1]:
>     Anyway, every single __get_user() call I looked at looked like
>     historical garbage. [...] End result: I get the feeling that we
>     should just do a global search-and-replace of the __get_user/
>     __put_user users, replace them with plain get_user/put_user instead,
>     and then fix up any fallout (eg the coco code).
>
> Switch to plain get_user/put_user in vhost, which results in a slight
> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
>
> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
> RX: taskset -c 2 iperf3 -s -p 5200 -D
> Before: 6.08 Gbits/sec
> After:  6.32 Gbits/sec

I wonder if we need to test on archs like ARM.

Thanks


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-13  1:09 ` Jason Wang
@ 2025-11-14 14:53   ` Jon Kohler
  2025-11-14 17:48     ` Linus Torvalds
  2025-11-17  4:32     ` Jason Wang
  0 siblings, 2 replies; 27+ messages in thread
From: Jon Kohler @ 2025-11-14 14:53 UTC (permalink / raw)
  To: Jason Wang
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson



> On Nov 12, 2025, at 8:09 PM, Jason Wang <jasowang@redhat.com> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> On Thu, Nov 13, 2025 at 8:14 AM Jon Kohler <jon@nutanix.com> wrote:
>> 
>> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
>> respectively, which were both added in 2016 by commit 6b1e6cc7855b
>> ("vhost: new device IOTLB API").
> 
> It has been used even before this commit.

Ah, thanks for the pointer. I’d have to go dig to find its genesis, but
its more to say, this existed prior to the LFENCE commit.

> 
>> In a heavy UDP transmit workload on a
>> vhost-net backed tap device, these functions showed up as ~11.6% of
>> samples in a flamegraph of the underlying vhost worker thread.
>> 
>> Quoting Linus from [1]:
>>    Anyway, every single __get_user() call I looked at looked like
>>    historical garbage. [...] End result: I get the feeling that we
>>    should just do a global search-and-replace of the __get_user/
>>    __put_user users, replace them with plain get_user/put_user instead,
>>    and then fix up any fallout (eg the coco code).
>> 
>> Switch to plain get_user/put_user in vhost, which results in a slight
>> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
>> 
>> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
>> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
>> RX: taskset -c 2 iperf3 -s -p 5200 -D
>> Before: 6.08 Gbits/sec
>> After:  6.32 Gbits/sec
> 
> I wonder if we need to test on archs like ARM.

Are you thinking from a performance perspective? Or a correctness one?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 14:53   ` Jon Kohler
@ 2025-11-14 17:48     ` Linus Torvalds
  2025-11-14 19:08       ` David Laight
  2025-11-17  4:32     ` Jason Wang
  1 sibling, 1 reply; 27+ messages in thread
From: Linus Torvalds @ 2025-11-14 17:48 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Borislav Petkov, Sean Christopherson

On Fri, 14 Nov 2025 at 06:53, Jon Kohler <jon@nutanix.com> wrote:
>
> > On Nov 12, 2025, at 8:09 PM, Jason Wang <jasowang@redhat.com> wrote:
> >
> > It has been used even before this commit.
>
> Ah, thanks for the pointer. I’d have to go dig to find its genesis, but
> its more to say, this existed prior to the LFENCE commit.

It might still be worth pointing out that the LFENCE is what made
__get_user() and friends much slower, but the *real* issue is that
__get_user() and friends became *pointless* before that due to SMAP.

Because the whole - and only - point of the __get_user() interface is
the historical issue of "it translates to a single load instruction
and gets inlined".

So back in the dark ages before SMAP, a regular "get_user()" was a
function call and maybe six instructions worth over code. But a
"__get_user()" was inlined to be a single instruction, and the
difference between the two was quite noticeable if you did things in a
loop.

End result: we have a number of old historic __get_user() calls
because people cared and it was noticeable.

But then SMAP happens, and user space accesses aren't just a simple
single load instruction, but a "enable user space access, do the
access, then disable user space accesses" for safety and robustness
reasons.

That's actually true on non-x86 architectures too: on arm64 you also
have TTBR0_PAN, and user space accesses can be quite the mess of
instructions.

And in that whole change, now __get_user() is not only no longer
inlined, the performance advantage isn't relevant any more. Sure, it
still avoided the user address space range check, but that check just
is no longer relevant. It's a couple of (very cheap) instructions, but
the big reason to use __get_user() has simply gone away. The real
costs of user space accesses are elsewhere, and __get_user() and
get_user() are basically the same performance in reality.

But the historical uses of __get_user() remain, even though now they
are pretty much pointless.

Then LFENCE comes around due to the whole speculation issue, and
initially __get_user() and get_user() *both* get it, and it only
reinforces the whole "the address check is not the most expensive part
of the operation" thing, but __get_user() is still technically a cycle
or two faster.

But then get_user() gets optimized to do the address space check using
a data dependency instead of the "access_ok()" control dependency, and
so get_user() doesn't need LFENCE at all, and now get_user() is
*faster* than __get_user().

End result: __get_user() has been a historical artifact for a long
time. It's been discouraged because it's pointless. But with the
LFENCE it's not only pointless, it's actively detrimental not just for
safety but even for performance.

So __get_user() should die. The LFENCE is not the primary reason it
should be retired, it's only the last nail in the coffin.

             Linus

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-13  0:55 [PATCH net-next] vhost: use "checked" versions of get_user() and put_user() Jon Kohler
  2025-11-13  1:09 ` Jason Wang
@ 2025-11-14 18:54 ` David Laight
  2025-11-14 19:30   ` Jon Kohler
  1 sibling, 1 reply; 27+ messages in thread
From: David Laight @ 2025-11-14 18:54 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Michael S. Tsirkin, Jason Wang, Eugenio Pérez, kvm,
	virtualization, netdev, linux-kernel, Linus Torvalds,
	Borislav Petkov, Sean Christopherson

On Wed, 12 Nov 2025 17:55:28 -0700
Jon Kohler <jon@nutanix.com> wrote:

> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
> respectively, which were both added in 2016 by commit 6b1e6cc7855b
> ("vhost: new device IOTLB API"). In a heavy UDP transmit workload on a
> vhost-net backed tap device, these functions showed up as ~11.6% of
> samples in a flamegraph of the underlying vhost worker thread.
> 
> Quoting Linus from [1]:
>     Anyway, every single __get_user() call I looked at looked like
>     historical garbage. [...] End result: I get the feeling that we
>     should just do a global search-and-replace of the __get_user/
>     __put_user users, replace them with plain get_user/put_user instead,
>     and then fix up any fallout (eg the coco code).
> 
> Switch to plain get_user/put_user in vhost, which results in a slight
> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
> 
> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
> RX: taskset -c 2 iperf3 -s -p 5200 -D
> Before: 6.08 Gbits/sec
> After:  6.32 Gbits/sec
> 
> As to what drives the speedup, Sean's patch [2] explains:
> 	Use the normal, checked versions for get_user() and put_user() instead of
> 	the double-underscore versions that omit range checks, as the checked
> 	versions are actually measurably faster on modern CPUs (12%+ on Intel,
> 	25%+ on AMD).

Is there an associated access_ok() that can also be removed?

	David

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 17:48     ` Linus Torvalds
@ 2025-11-14 19:08       ` David Laight
  2025-11-14 20:48         ` Linus Torvalds
  0 siblings, 1 reply; 27+ messages in thread
From: David Laight @ 2025-11-14 19:08 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Jon Kohler, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Borislav Petkov, Sean Christopherson

On Fri, 14 Nov 2025 09:48:02 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

...
> But then get_user() gets optimized to do the address space check using
> a data dependency instead of the "access_ok()" control dependency, and
> so get_user() doesn't need LFENCE at all, and now get_user() is
> *faster* than __get_user().

I think that is currently only x86-64?
There are patches in the pipeline for ppc.
I don't think I've seen anything for arm32 or arm64.

arm64 has the issue that the hardware looks at the wrong address bit,
so might need an explicit guard page at the end of user addresses.

Changing x86-32 to have a guard page ought to be straightforward.
But I think the user stack ends right at 0xc000000 (with argv[] and env[])
so it might be safer to also reduce the stack size by 4k (pretending
env[] is larger) to avoid problems with code that is trying to map
things at fixed addresses just below the stack (or do we care about that?).

I'm sure I should be able to build and test the x86-32 code.
I guess there are instruction for doing that under qemu somewhere?
Might be time to drop support for cpu that don't support cmov?

	David

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 18:54 ` David Laight
@ 2025-11-14 19:30   ` Jon Kohler
  2025-11-14 20:32     ` David Laight
  2025-11-16  6:32     ` Michael S. Tsirkin
  0 siblings, 2 replies; 27+ messages in thread
From: Jon Kohler @ 2025-11-14 19:30 UTC (permalink / raw)
  To: David Laight
  Cc: Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linus Torvalds, Borislav Petkov, Sean Christopherson



> On Nov 14, 2025, at 1:54 PM, David Laight <david.laight.linux@gmail.com> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> On Wed, 12 Nov 2025 17:55:28 -0700
> Jon Kohler <jon@nutanix.com> wrote:
> 
>> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
>> respectively, which were both added in 2016 by commit 6b1e6cc7855b
>> ("vhost: new device IOTLB API"). In a heavy UDP transmit workload on a
>> vhost-net backed tap device, these functions showed up as ~11.6% of
>> samples in a flamegraph of the underlying vhost worker thread.
>> 
>> Quoting Linus from [1]:
>>    Anyway, every single __get_user() call I looked at looked like
>>    historical garbage. [...] End result: I get the feeling that we
>>    should just do a global search-and-replace of the __get_user/
>>    __put_user users, replace them with plain get_user/put_user instead,
>>    and then fix up any fallout (eg the coco code).
>> 
>> Switch to plain get_user/put_user in vhost, which results in a slight
>> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
>> 
>> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
>> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
>> RX: taskset -c 2 iperf3 -s -p 5200 -D
>> Before: 6.08 Gbits/sec
>> After:  6.32 Gbits/sec
>> 
>> As to what drives the speedup, Sean's patch [2] explains:
>> Use the normal, checked versions for get_user() and put_user() instead of
>> the double-underscore versions that omit range checks, as the checked
>> versions are actually measurably faster on modern CPUs (12%+ on Intel,
>> 25%+ on AMD).
> 
> Is there an associated access_ok() that can also be removed?
> 
> David

Hey David - IIUC, the access_ok() for non-iotlb setups is done at
initial setup time, not per event, see vhost_vring_set_addr and
for the vhost net side see vhost_net_set_backend -> 
vhost_vq_access_ok.

Will lean on MST/Jason to help sanity check my understanding.

In the iotlb case, that’s handled differently (Jason can speak to
that side), but I dont think there is something we’d remove there?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 19:30   ` Jon Kohler
@ 2025-11-14 20:32     ` David Laight
  2025-11-16  6:32     ` Michael S. Tsirkin
  1 sibling, 0 replies; 27+ messages in thread
From: David Laight @ 2025-11-14 20:32 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linus Torvalds, Borislav Petkov, Sean Christopherson

On Fri, 14 Nov 2025 19:30:32 +0000
Jon Kohler <jon@nutanix.com> wrote:

> > On Nov 14, 2025, at 1:54 PM, David Laight <david.laight.linux@gmail.com> wrote:
> > 
> > !-------------------------------------------------------------------|
> >  CAUTION: External Email
> > 
> > |-------------------------------------------------------------------!
> > 
> > On Wed, 12 Nov 2025 17:55:28 -0700
> > Jon Kohler <jon@nutanix.com> wrote:
> >   
> >> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
> >> respectively, which were both added in 2016 by commit 6b1e6cc7855b
> >> ("vhost: new device IOTLB API"). In a heavy UDP transmit workload on a
> >> vhost-net backed tap device, these functions showed up as ~11.6% of
> >> samples in a flamegraph of the underlying vhost worker thread.
> >> 
> >> Quoting Linus from [1]:
> >>    Anyway, every single __get_user() call I looked at looked like
> >>    historical garbage. [...] End result: I get the feeling that we
> >>    should just do a global search-and-replace of the __get_user/
> >>    __put_user users, replace them with plain get_user/put_user instead,
> >>    and then fix up any fallout (eg the coco code).
> >> 
> >> Switch to plain get_user/put_user in vhost, which results in a slight
> >> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
> >> 
> >> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
> >> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
> >> RX: taskset -c 2 iperf3 -s -p 5200 -D
> >> Before: 6.08 Gbits/sec
> >> After:  6.32 Gbits/sec
> >> 
> >> As to what drives the speedup, Sean's patch [2] explains:
> >> Use the normal, checked versions for get_user() and put_user() instead of
> >> the double-underscore versions that omit range checks, as the checked
> >> versions are actually measurably faster on modern CPUs (12%+ on Intel,
> >> 25%+ on AMD).  
> > 
> > Is there an associated access_ok() that can also be removed?
> > 
> > David  
> 
> Hey David - IIUC, the access_ok() for non-iotlb setups is done at
> initial setup time, not per event, see vhost_vring_set_addr and
> for the vhost net side see vhost_net_set_backend -> 
> vhost_vq_access_ok.

This is a long way away from the actual access....

The early 'sanity check' might be worth keeping, but the code has to
allow for the user access faulting (the application might unmap it).
But, in some sense, that early check is optimising for the user passing
in an invalid buffer - so not actually worth while,

> Will lean on MST/Jason to help sanity check my understanding.
> 
> In the iotlb case, that’s handled differently (Jason can speak to
> that side), but I dont think there is something we’d remove there?

Isn't the application side much the same?
(But I don't know what the code is doing...)

	David


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 19:08       ` David Laight
@ 2025-11-14 20:48         ` Linus Torvalds
  2025-11-14 21:38           ` David Laight
  0 siblings, 1 reply; 27+ messages in thread
From: Linus Torvalds @ 2025-11-14 20:48 UTC (permalink / raw)
  To: David Laight
  Cc: Jon Kohler, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Borislav Petkov, Sean Christopherson

On Fri, 14 Nov 2025 at 11:09, David Laight <david.laight.linux@gmail.com> wrote:
>
> I think that is currently only x86-64?
> There are patches in the pipeline for ppc.
> I don't think I've seen anything for arm32 or arm64.

Honestly, the fact that it's mainly true on x86-64 is simply because
that's the only architecture that has cared enough.

Pretty much everybody else is affected by the exact same speculation
bugs. Sometimes the speculation window might be so small that it
doesn't matter, but in most cases it's just that the architecture is
so irrelevant that it doesn't matter.

So no, this is not a "x86 only" issue. It might be a "only a couple of
architectures have cared enough for it to have any practical impact".

End result: if some other architecture still has a __get_user() that
is noticeably faster than get_user(), it's not an argument for keeping
__get_user() - it's an argument that that architecture likely isn't
very important.

           Linus

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 20:48         ` Linus Torvalds
@ 2025-11-14 21:38           ` David Laight
  0 siblings, 0 replies; 27+ messages in thread
From: David Laight @ 2025-11-14 21:38 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Jon Kohler, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Borislav Petkov, Sean Christopherson

On Fri, 14 Nov 2025 12:48:52 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, 14 Nov 2025 at 11:09, David Laight <david.laight.linux@gmail.com> wrote:
> >
> > I think that is currently only x86-64?
> > There are patches in the pipeline for ppc.
> > I don't think I've seen anything for arm32 or arm64.  
> 
> Honestly, the fact that it's mainly true on x86-64 is simply because
> that's the only architecture that has cared enough.
> 
> Pretty much everybody else is affected by the exact same speculation
> bugs. Sometimes the speculation window might be so small that it
> doesn't matter, but in most cases it's just that the architecture is
> so irrelevant that it doesn't matter.
> 
> So no, this is not a "x86 only" issue. It might be a "only a couple of
> architectures have cared enough for it to have any practical impact".
> 
> End result: if some other architecture still has a __get_user() that
> is noticeably faster than get_user(), it's not an argument for keeping
> __get_user() - it's an argument that that architecture likely isn't
> very important.

I was really thinking it was a justification to get the 'address masking'
implemented for other architectures.

It wouldn't surprise me if some of the justifications for the 'guard page'
at the top of x86-64 userspace (like speculative execution across the
user-kernel boundary) aren't a more general problem.

So adding support to arm32, arm64, riscV and 32bit x86 might be reasonable.
What does that really leave? sparc, m68k?

At that point requiring a guard page for all architectures starts looking
reasonable, and the non 'address masking' user access checks can all be
thrown away.
That isn't going to happen quickly, but seems a reasonable aim.

Architectures without speculation issues (old ones) can use a C compare.
I think this works for 32bit x86 (without cmov):
	mov $-guard_page, %guard_off
	add %user_addr, %guard_off
	sbb %mask, %mask
	and %mask, %guard_off
	sub %guard_off, %user_addr
mips-like architectures (no flags) probably require a 'cmp' and 'dec'
to generate the mask value.
(I'm not sure how that compares to any of the ppc asm blocks.)

	David

> 
>            Linus


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 19:30   ` Jon Kohler
  2025-11-14 20:32     ` David Laight
@ 2025-11-16  6:32     ` Michael S. Tsirkin
  1 sibling, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2025-11-16  6:32 UTC (permalink / raw)
  To: Jon Kohler
  Cc: David Laight, Jason Wang, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson

On Fri, Nov 14, 2025 at 07:30:32PM +0000, Jon Kohler wrote:
> 
> 
> > On Nov 14, 2025, at 1:54 PM, David Laight <david.laight.linux@gmail.com> wrote:
> > 
> > !-------------------------------------------------------------------|
> >  CAUTION: External Email
> > 
> > |-------------------------------------------------------------------!
> > 
> > On Wed, 12 Nov 2025 17:55:28 -0700
> > Jon Kohler <jon@nutanix.com> wrote:
> > 
> >> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
> >> respectively, which were both added in 2016 by commit 6b1e6cc7855b
> >> ("vhost: new device IOTLB API"). In a heavy UDP transmit workload on a
> >> vhost-net backed tap device, these functions showed up as ~11.6% of
> >> samples in a flamegraph of the underlying vhost worker thread.
> >> 
> >> Quoting Linus from [1]:
> >>    Anyway, every single __get_user() call I looked at looked like
> >>    historical garbage. [...] End result: I get the feeling that we
> >>    should just do a global search-and-replace of the __get_user/
> >>    __put_user users, replace them with plain get_user/put_user instead,
> >>    and then fix up any fallout (eg the coco code).
> >> 
> >> Switch to plain get_user/put_user in vhost, which results in a slight
> >> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
> >> 
> >> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
> >> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
> >> RX: taskset -c 2 iperf3 -s -p 5200 -D
> >> Before: 6.08 Gbits/sec
> >> After:  6.32 Gbits/sec
> >> 
> >> As to what drives the speedup, Sean's patch [2] explains:
> >> Use the normal, checked versions for get_user() and put_user() instead of
> >> the double-underscore versions that omit range checks, as the checked
> >> versions are actually measurably faster on modern CPUs (12%+ on Intel,
> >> 25%+ on AMD).
> > 
> > Is there an associated access_ok() that can also be removed?
> > 
> > David
> 
> Hey David - IIUC, the access_ok() for non-iotlb setups is done at
> initial setup time, not per event, see vhost_vring_set_addr and
> for the vhost net side see vhost_net_set_backend -> 
> vhost_vq_access_ok.
> 
> Will lean on MST/Jason to help sanity check my understanding.

Right.

> In the iotlb case, that’s handled differently (Jason can speak to
> that side), but I dont think there is something we’d remove there?


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-14 14:53   ` Jon Kohler
  2025-11-14 17:48     ` Linus Torvalds
@ 2025-11-17  4:32     ` Jason Wang
  2025-11-17 17:34       ` Jon Kohler
  1 sibling, 1 reply; 27+ messages in thread
From: Jason Wang @ 2025-11-17  4:32 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson

On Fri, Nov 14, 2025 at 10:53 PM Jon Kohler <jon@nutanix.com> wrote:
>
>
>
> > On Nov 12, 2025, at 8:09 PM, Jason Wang <jasowang@redhat.com> wrote:
> >
> > !-------------------------------------------------------------------|
> >  CAUTION: External Email
> >
> > |-------------------------------------------------------------------!
> >
> > On Thu, Nov 13, 2025 at 8:14 AM Jon Kohler <jon@nutanix.com> wrote:
> >>
> >> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
> >> respectively, which were both added in 2016 by commit 6b1e6cc7855b
> >> ("vhost: new device IOTLB API").
> >
> > It has been used even before this commit.
>
> Ah, thanks for the pointer. I’d have to go dig to find its genesis, but
> its more to say, this existed prior to the LFENCE commit.
>
> >
> >> In a heavy UDP transmit workload on a
> >> vhost-net backed tap device, these functions showed up as ~11.6% of
> >> samples in a flamegraph of the underlying vhost worker thread.
> >>
> >> Quoting Linus from [1]:
> >>    Anyway, every single __get_user() call I looked at looked like
> >>    historical garbage. [...] End result: I get the feeling that we
> >>    should just do a global search-and-replace of the __get_user/
> >>    __put_user users, replace them with plain get_user/put_user instead,
> >>    and then fix up any fallout (eg the coco code).
> >>
> >> Switch to plain get_user/put_user in vhost, which results in a slight
> >> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
> >>
> >> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
> >> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
> >> RX: taskset -c 2 iperf3 -s -p 5200 -D
> >> Before: 6.08 Gbits/sec
> >> After:  6.32 Gbits/sec
> >
> > I wonder if we need to test on archs like ARM.
>
> Are you thinking from a performance perspective? Or a correctness one?

Performance, I think the patch is correct.

Thanks


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-17  4:32     ` Jason Wang
@ 2025-11-17 17:34       ` Jon Kohler
  2025-11-20  1:57         ` Jason Wang
  0 siblings, 1 reply; 27+ messages in thread
From: Jon Kohler @ 2025-11-17 17:34 UTC (permalink / raw)
  To: Jason Wang
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson



> On Nov 16, 2025, at 11:32 PM, Jason Wang <jasowang@redhat.com> wrote:
> 
> On Fri, Nov 14, 2025 at 10:53 PM Jon Kohler <jon@nutanix.com> wrote:
>> 
>> 
>> 
>>> On Nov 12, 2025, at 8:09 PM, Jason Wang <jasowang@redhat.com> wrote:
>>> 
>>> !-------------------------------------------------------------------|
>>> CAUTION: External Email
>>> 
>>> |-------------------------------------------------------------------!
>>> 
>>> On Thu, Nov 13, 2025 at 8:14 AM Jon Kohler <jon@nutanix.com> wrote:
>>>> 
>>>> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
>>>> respectively, which were both added in 2016 by commit 6b1e6cc7855b
>>>> ("vhost: new device IOTLB API").
>>> 
>>> It has been used even before this commit.
>> 
>> Ah, thanks for the pointer. I’d have to go dig to find its genesis, but
>> its more to say, this existed prior to the LFENCE commit.
>> 
>>> 
>>>> In a heavy UDP transmit workload on a
>>>> vhost-net backed tap device, these functions showed up as ~11.6% of
>>>> samples in a flamegraph of the underlying vhost worker thread.
>>>> 
>>>> Quoting Linus from [1]:
>>>>   Anyway, every single __get_user() call I looked at looked like
>>>>   historical garbage. [...] End result: I get the feeling that we
>>>>   should just do a global search-and-replace of the __get_user/
>>>>   __put_user users, replace them with plain get_user/put_user instead,
>>>>   and then fix up any fallout (eg the coco code).
>>>> 
>>>> Switch to plain get_user/put_user in vhost, which results in a slight
>>>> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
>>>> 
>>>> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
>>>> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
>>>> RX: taskset -c 2 iperf3 -s -p 5200 -D
>>>> Before: 6.08 Gbits/sec
>>>> After:  6.32 Gbits/sec
>>> 
>>> I wonder if we need to test on archs like ARM.
>> 
>> Are you thinking from a performance perspective? Or a correctness one?
> 
> Performance, I think the patch is correct.
> 
> Thanks
> 

Ok gotcha. If anyone has an ARM system stuffed in their
front pocket and can give this a poke, I’d appreciate it, as
I don’t have ready access to one personally.

That said, I think this might end up in “well, it is what it is”
territory as Linus was alluding to, i.e. if performance dips on
ARM for vhost, then thats a compelling point to optimize whatever
ends up being the culprit for get/put user?

Said another way, would ARM perf testing (or any other arch) be a
blocker to taking this change?

Thanks - Jon


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-17 17:34       ` Jon Kohler
@ 2025-11-20  1:57         ` Jason Wang
  2025-11-25 19:45           ` Jon Kohler
  0 siblings, 1 reply; 27+ messages in thread
From: Jason Wang @ 2025-11-20  1:57 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson

On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
>
>
>
> > On Nov 16, 2025, at 11:32 PM, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, Nov 14, 2025 at 10:53 PM Jon Kohler <jon@nutanix.com> wrote:
> >>
> >>
> >>
> >>> On Nov 12, 2025, at 8:09 PM, Jason Wang <jasowang@redhat.com> wrote:
> >>>
> >>> !-------------------------------------------------------------------|
> >>> CAUTION: External Email
> >>>
> >>> |-------------------------------------------------------------------!
> >>>
> >>> On Thu, Nov 13, 2025 at 8:14 AM Jon Kohler <jon@nutanix.com> wrote:
> >>>>
> >>>> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
> >>>> respectively, which were both added in 2016 by commit 6b1e6cc7855b
> >>>> ("vhost: new device IOTLB API").
> >>>
> >>> It has been used even before this commit.
> >>
> >> Ah, thanks for the pointer. I’d have to go dig to find its genesis, but
> >> its more to say, this existed prior to the LFENCE commit.
> >>
> >>>
> >>>> In a heavy UDP transmit workload on a
> >>>> vhost-net backed tap device, these functions showed up as ~11.6% of
> >>>> samples in a flamegraph of the underlying vhost worker thread.
> >>>>
> >>>> Quoting Linus from [1]:
> >>>>   Anyway, every single __get_user() call I looked at looked like
> >>>>   historical garbage. [...] End result: I get the feeling that we
> >>>>   should just do a global search-and-replace of the __get_user/
> >>>>   __put_user users, replace them with plain get_user/put_user instead,
> >>>>   and then fix up any fallout (eg the coco code).
> >>>>
> >>>> Switch to plain get_user/put_user in vhost, which results in a slight
> >>>> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
> >>>>
> >>>> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
> >>>> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
> >>>> RX: taskset -c 2 iperf3 -s -p 5200 -D
> >>>> Before: 6.08 Gbits/sec
> >>>> After:  6.32 Gbits/sec
> >>>
> >>> I wonder if we need to test on archs like ARM.
> >>
> >> Are you thinking from a performance perspective? Or a correctness one?
> >
> > Performance, I think the patch is correct.
> >
> > Thanks
> >
>
> Ok gotcha. If anyone has an ARM system stuffed in their
> front pocket and can give this a poke, I’d appreciate it, as
> I don’t have ready access to one personally.
>
> That said, I think this might end up in “well, it is what it is”
> territory as Linus was alluding to, i.e. if performance dips on
> ARM for vhost, then thats a compelling point to optimize whatever
> ends up being the culprit for get/put user?
>
> Said another way, would ARM perf testing (or any other arch) be a
> blocker to taking this change?

Not a must but at least we need to explain the implication for other
archs as the discussion you quoted are all for x86.

Thanks

>
> Thanks - Jon
>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-20  1:57         ` Jason Wang
@ 2025-11-25 19:45           ` Jon Kohler
  2025-11-26  6:04             ` Jason Wang
  0 siblings, 1 reply; 27+ messages in thread
From: Jon Kohler @ 2025-11-25 19:45 UTC (permalink / raw)
  To: Jason Wang
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson



> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
> 
> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
>> 
>> 
>>> On Nov 16, 2025, at 11:32 PM, Jason Wang <jasowang@redhat.com> wrote:
>>> 
>>> On Fri, Nov 14, 2025 at 10:53 PM Jon Kohler <jon@nutanix.com> wrote:
>>>> 
>>>> 
>>>>> On Nov 12, 2025, at 8:09 PM, Jason Wang <jasowang@redhat.com> wrote:
>>>>> 
>>>>> On Thu, Nov 13, 2025 at 8:14 AM Jon Kohler <jon@nutanix.com> wrote:
>>>>>> 
>>>>>> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
>>>>>> respectively, which were both added in 2016 by commit 6b1e6cc7855b
>>>>>> ("vhost: new device IOTLB API").
>>>>> 
>>>>> It has been used even before this commit.
>>>> 
>>>> Ah, thanks for the pointer. I’d have to go dig to find its genesis, but
>>>> its more to say, this existed prior to the LFENCE commit.
>>>> 
>>>>> 
>>>>>> In a heavy UDP transmit workload on a
>>>>>> vhost-net backed tap device, these functions showed up as ~11.6% of
>>>>>> samples in a flamegraph of the underlying vhost worker thread.
>>>>>> 
>>>>>> Quoting Linus from [1]:
>>>>>>  Anyway, every single __get_user() call I looked at looked like
>>>>>>  historical garbage. [...] End result: I get the feeling that we
>>>>>>  should just do a global search-and-replace of the __get_user/
>>>>>>  __put_user users, replace them with plain get_user/put_user instead,
>>>>>>  and then fix up any fallout (eg the coco code).
>>>>>> 
>>>>>> Switch to plain get_user/put_user in vhost, which results in a slight
>>>>>> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
>>>>>> 
>>>>>> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
>>>>>> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
>>>>>> RX: taskset -c 2 iperf3 -s -p 5200 -D
>>>>>> Before: 6.08 Gbits/sec
>>>>>> After:  6.32 Gbits/sec
>>>>> 
>>>>> I wonder if we need to test on archs like ARM.
>>>> 
>>>> Are you thinking from a performance perspective? Or a correctness one?
>>> 
>>> Performance, I think the patch is correct.
>>> 
>>> Thanks
>>> 
>> 
>> Ok gotcha. If anyone has an ARM system stuffed in their
>> front pocket and can give this a poke, I’d appreciate it, as
>> I don’t have ready access to one personally.
>> 
>> That said, I think this might end up in “well, it is what it is”
>> territory as Linus was alluding to, i.e. if performance dips on
>> ARM for vhost, then thats a compelling point to optimize whatever
>> ends up being the culprit for get/put user?
>> 
>> Said another way, would ARM perf testing (or any other arch) be a
>> blocker to taking this change?
> 
> Not a must but at least we need to explain the implication for other
> archs as the discussion you quoted are all for x86.
> 
> Thanks

I’ll admit my ARM muscle is weak, but here’s my best take on this: 

Looking at arch/arm/include/asm/uaccess.h, the biggest thing that I
noticed is the CONFIG_CPU_SPECTRE ifdef, which already remaps
__get_user() to get_user(), so anyone running that in their kconfig
will already practically have the behavior implemented by this patch
by way of commit b1cd0a148063 ("ARM: spectre-v1: use get_user() for
__get_user()”).

Same deal goes for __put_user() vs put_user by way of commit
e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)

Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at 
commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
about everything ARM would include this by default? 

If so, that mean at least for a non-zero population of ARM’ers,
they wouldn’t notice anything from this patch, yea?

Happy to learn otherwise if that read is incorrect! 

Thanks all,
Jon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-25 19:45           ` Jon Kohler
@ 2025-11-26  6:04             ` Jason Wang
  2025-11-26 10:25               ` Arnd Bergmann
  0 siblings, 1 reply; 27+ messages in thread
From: Jason Wang @ 2025-11-26  6:04 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel, Russell King - ARM Linux,
	Catalin Marinas, Will Deacon, Arnd Bergmann, krzk,
	alexandre.belloni, Linus Walleij, fustini

On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
>
>
>
> > On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
> >>
> >>
> >>> On Nov 16, 2025, at 11:32 PM, Jason Wang <jasowang@redhat.com> wrote:
> >>>
> >>> On Fri, Nov 14, 2025 at 10:53 PM Jon Kohler <jon@nutanix.com> wrote:
> >>>>
> >>>>
> >>>>> On Nov 12, 2025, at 8:09 PM, Jason Wang <jasowang@redhat.com> wrote:
> >>>>>
> >>>>> On Thu, Nov 13, 2025 at 8:14 AM Jon Kohler <jon@nutanix.com> wrote:
> >>>>>>
> >>>>>> vhost_get_user and vhost_put_user leverage __get_user and __put_user,
> >>>>>> respectively, which were both added in 2016 by commit 6b1e6cc7855b
> >>>>>> ("vhost: new device IOTLB API").
> >>>>>
> >>>>> It has been used even before this commit.
> >>>>
> >>>> Ah, thanks for the pointer. I’d have to go dig to find its genesis, but
> >>>> its more to say, this existed prior to the LFENCE commit.
> >>>>
> >>>>>
> >>>>>> In a heavy UDP transmit workload on a
> >>>>>> vhost-net backed tap device, these functions showed up as ~11.6% of
> >>>>>> samples in a flamegraph of the underlying vhost worker thread.
> >>>>>>
> >>>>>> Quoting Linus from [1]:
> >>>>>>  Anyway, every single __get_user() call I looked at looked like
> >>>>>>  historical garbage. [...] End result: I get the feeling that we
> >>>>>>  should just do a global search-and-replace of the __get_user/
> >>>>>>  __put_user users, replace them with plain get_user/put_user instead,
> >>>>>>  and then fix up any fallout (eg the coco code).
> >>>>>>
> >>>>>> Switch to plain get_user/put_user in vhost, which results in a slight
> >>>>>> throughput speedup. get_user now about ~8.4% of samples in flamegraph.
> >>>>>>
> >>>>>> Basic iperf3 test on a Intel 5416S CPU with Ubuntu 25.10 guest:
> >>>>>> TX: taskset -c 2 iperf3 -c <rx_ip> -t 60 -p 5200 -b 0 -u -i 5
> >>>>>> RX: taskset -c 2 iperf3 -s -p 5200 -D
> >>>>>> Before: 6.08 Gbits/sec
> >>>>>> After:  6.32 Gbits/sec
> >>>>>
> >>>>> I wonder if we need to test on archs like ARM.
> >>>>
> >>>> Are you thinking from a performance perspective? Or a correctness one?
> >>>
> >>> Performance, I think the patch is correct.
> >>>
> >>> Thanks
> >>>
> >>
> >> Ok gotcha. If anyone has an ARM system stuffed in their
> >> front pocket and can give this a poke, I’d appreciate it, as
> >> I don’t have ready access to one personally.
> >>
> >> That said, I think this might end up in “well, it is what it is”
> >> territory as Linus was alluding to, i.e. if performance dips on
> >> ARM for vhost, then thats a compelling point to optimize whatever
> >> ends up being the culprit for get/put user?
> >>
> >> Said another way, would ARM perf testing (or any other arch) be a
> >> blocker to taking this change?
> >
> > Not a must but at least we need to explain the implication for other
> > archs as the discussion you quoted are all for x86.
> >
> > Thanks
>
> I’ll admit my ARM muscle is weak, but here’s my best take on this:
>
> Looking at arch/arm/include/asm/uaccess.h, the biggest thing that I
> noticed is the CONFIG_CPU_SPECTRE ifdef, which already remaps
> __get_user() to get_user(), so anyone running that in their kconfig
> will already practically have the behavior implemented by this patch
> by way of commit b1cd0a148063 ("ARM: spectre-v1: use get_user() for
> __get_user()”).
>
> Same deal goes for __put_user() vs put_user by way of commit
> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
>
> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
> about everything ARM would include this by default?
>
> If so, that mean at least for a non-zero population of ARM’ers,
> they wouldn’t notice anything from this patch, yea?

Adding ARM maintainers for more thought.

Thanks

>
> Happy to learn otherwise if that read is incorrect!
>
> Thanks all,
> Jon


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-26  6:04             ` Jason Wang
@ 2025-11-26 10:25               ` Arnd Bergmann
  2025-11-26 19:47                 ` Jon Kohler
  0 siblings, 1 reply; 27+ messages in thread
From: Arnd Bergmann @ 2025-11-26 10:25 UTC (permalink / raw)
  To: Jason Wang, Jon Kohler
  Cc: Michael S. Tsirkin, Eugenio Pérez, kvm@vger.kernel.org,
	virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel, Russell King,
	Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini

On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
>> > On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
>> > On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
>> Same deal goes for __put_user() vs put_user by way of commit
>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
>>
>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
>> about everything ARM would include this by default?

I think the more relevant commit is for 64-bit Arm here, but this does
the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
eliding access_ok checks in __{get, put}_user").

Note that there is no KVM on 32-bit Arm any more, so we really don't
care about vhost performance there. The added access_ok() check in
arm32 __get_user() is probably avoidable, as embedded systems with
in-order cores could turn off the spectre workarounds, but as
Will explained in the arm64 commit, it's not that expensive either.

>> If so, that mean at least for a non-zero population of ARM’ers,
>> they wouldn’t notice anything from this patch, yea?
>
> Adding ARM maintainers for more thought.

I would think that if we change the __get_user() to get_user()
in this driver, the same should be done for the
__copy_{from,to}_user(), which similarly skips the access_ok()
check but not the PAN/SMAP handling.

In general, the access_ok()/__get_user()/__copy_from_user()
pattern isn't really helpful any more, as Linus already
explained. I can't tell from the vhost driver code whether
we can just drop the access_ok() here and use the plain
get_user()/copy_from_user(), or if it makes sense to move
to the newer user_access_begin()/unsafe_get_user()/
unsafe_copy_from_user()/user_access_end() and try optimize
out a few PAN/SMAP flips in the process.

     Arnd

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-26 10:25               ` Arnd Bergmann
@ 2025-11-26 19:47                 ` Jon Kohler
  2025-11-26 19:58                   ` Arnd Bergmann
  2025-11-27  1:08                   ` Jason Wang
  0 siblings, 2 replies; 27+ messages in thread
From: Jon Kohler @ 2025-11-26 19:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini



> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
>>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
>>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
>>> Same deal goes for __put_user() vs put_user by way of commit
>>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
>>> 
>>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
>>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
>>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
>>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
>>> about everything ARM would include this by default?
> 
> I think the more relevant commit is for 64-bit Arm here, but this does
> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
> eliding access_ok checks in __{get, put}_user").

Ah! Right, this is definitely the important bit, as it makes it
crystal clear that these are exactly the same thing. The current
code is:
#define get_user	__get_user
#define put_user	__put_user

So, this patch changing from __* to regular versions is a no-op
on arm side of the house, yea?

> I would think that if we change the __get_user() to get_user()
> in this driver, the same should be done for the
> __copy_{from,to}_user(), which similarly skips the access_ok()
> check but not the PAN/SMAP handling.

Perhaps, thats a good call out. I’d file that under one battle
at a time. Let’s get get/put user dusted first, then go down
that road?

> In general, the access_ok()/__get_user()/__copy_from_user()
> pattern isn't really helpful any more, as Linus already
> explained. I can't tell from the vhost driver code whether
> we can just drop the access_ok() here and use the plain
> get_user()/copy_from_user(), or if it makes sense to move
> to the newer user_access_begin()/unsafe_get_user()/
> unsafe_copy_from_user()/user_access_end() and try optimize
> out a few PAN/SMAP flips in the process.

In general, I think there are a few spots where we might be
able to optimize (vhost_get_vq_desc perhaps?) as that gets
called quite a bit and IIRC there are at least two flips
in there that perhaps we could elide to one? An investigation
for another day I think.

Anyhow, with this info - Jason - is there anything else you
can think of that we want to double click on?

Jon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-26 19:47                 ` Jon Kohler
@ 2025-11-26 19:58                   ` Arnd Bergmann
  2025-11-26 21:42                     ` Jon Kohler
  2025-11-27  1:08                   ` Jason Wang
  1 sibling, 1 reply; 27+ messages in thread
From: Arnd Bergmann @ 2025-11-26 19:58 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini

On Wed, Nov 26, 2025, at 20:47, Jon Kohler wrote:
>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
>> I think the more relevant commit is for 64-bit Arm here, but this does
>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
>> eliding access_ok checks in __{get, put}_user").
>
> Ah! Right, this is definitely the important bit, as it makes it
> crystal clear that these are exactly the same thing. The current
> code is:
> #define get_user	__get_user
> #define put_user	__put_user
>
> So, this patch changing from __* to regular versions is a no-op
> on arm side of the house, yea?

Certainly on 64-bit, and almost always on 32-bit, yes.

>> I would think that if we change the __get_user() to get_user()
>> in this driver, the same should be done for the
>> __copy_{from,to}_user(), which similarly skips the access_ok()
>> check but not the PAN/SMAP handling.
>
> Perhaps, thats a good call out. I’d file that under one battle
> at a time. Let’s get get/put user dusted first, then go down
> that road?

It depends on what your bigger plan is. Are you working on
improving the vhost driver specifically, or are you trying
to kill off the __get_user/__put_user calls across the
entire kernel?

In the latter case, I would suggest you do one driver
at a time but address access_ok(), __{get,put}_user and
__copy_{from,to}_user() with a single patch per driver
as long as this is simple enough. For vhost specifically,
doing it piecemeal is probably fine since the interaction
is more complicated than most others.

>> In general, the access_ok()/__get_user()/__copy_from_user()
>> pattern isn't really helpful any more, as Linus already
>> explained. I can't tell from the vhost driver code whether
>> we can just drop the access_ok() here and use the plain
>> get_user()/copy_from_user(), or if it makes sense to move
>> to the newer user_access_begin()/unsafe_get_user()/
>> unsafe_copy_from_user()/user_access_end() and try optimize
>> out a few PAN/SMAP flips in the process.
>
> In general, I think there are a few spots where we might be
> able to optimize (vhost_get_vq_desc perhaps?) as that gets
> called quite a bit and IIRC there are at least two flips
> in there that perhaps we could elide to one? An investigation
> for another day I think.

Yes, sounds good.

      Arnd

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-26 19:58                   ` Arnd Bergmann
@ 2025-11-26 21:42                     ` Jon Kohler
  2025-11-26 21:45                       ` Linus Torvalds
  0 siblings, 1 reply; 27+ messages in thread
From: Jon Kohler @ 2025-11-26 21:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini



> On Nov 26, 2025, at 2:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> On Wed, Nov 26, 2025, at 20:47, Jon Kohler wrote:
>>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
>>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
>>> I think the more relevant commit is for 64-bit Arm here, but this does
>>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
>>> eliding access_ok checks in __{get, put}_user").
>> 
>> Ah! Right, this is definitely the important bit, as it makes it
>> crystal clear that these are exactly the same thing. The current
>> code is:
>> #define get_user __get_user
>> #define put_user __put_user
>> 
>> So, this patch changing from __* to regular versions is a no-op
>> on arm side of the house, yea?
> 
> Certainly on 64-bit, and almost always on 32-bit, yes.
> 
>>> I would think that if we change the __get_user() to get_user()
>>> in this driver, the same should be done for the
>>> __copy_{from,to}_user(), which similarly skips the access_ok()
>>> check but not the PAN/SMAP handling.
>> 
>> Perhaps, thats a good call out. I’d file that under one battle
>> at a time. Let’s get get/put user dusted first, then go down
>> that road?
> 
> It depends on what your bigger plan is. Are you working on
> improving the vhost driver specifically, or are you trying
> to kill off the __get_user/__put_user calls across the
> entire kernel?

I’m working on vhost / virtualized networking improvements
at the moment, not the broader kernel wide work.

Linus mentioned he might get into the mix and do a bulk
change and kill the whole thing once and for all, so I’m
simply trying to help knock an incremental amount of work
off the pile in advance of that (and reap some performance
benefits at the same time, at least on the x86 side).

Thanks for the info and back n forth here, I appreciate it!

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-26 21:42                     ` Jon Kohler
@ 2025-11-26 21:45                       ` Linus Torvalds
  2025-11-27  2:58                         ` Jon Kohler
  0 siblings, 1 reply; 27+ messages in thread
From: Linus Torvalds @ 2025-11-26 21:45 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Arnd Bergmann, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini

On Wed, 26 Nov 2025 at 13:43, Jon Kohler <jon@nutanix.com> wrote:
>
> Linus mentioned he might get into the mix and do a bulk
> change and kill the whole thing once and for all, so I’m
> simply trying to help knock an incremental amount of work
> off the pile in advance of that (and reap some performance
> benefits at the same time, at least on the x86 side).

So I'm definitely going to do some bulk conversion at some point, but
honestly, I'll be a lot happier if most users already self-converted
before that, and I only end up doing a "convert unmaintained old code
that nobody really cares about".

                  Linus

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-26 19:47                 ` Jon Kohler
  2025-11-26 19:58                   ` Arnd Bergmann
@ 2025-11-27  1:08                   ` Jason Wang
  2025-11-27  3:11                     ` Jon Kohler
  1 sibling, 1 reply; 27+ messages in thread
From: Jason Wang @ 2025-11-27  1:08 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Arnd Bergmann, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini

On Thu, Nov 27, 2025 at 3:48 AM Jon Kohler <jon@nutanix.com> wrote:
>
>
>
> > On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
> >> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
> >>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
> >>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
> >>> Same deal goes for __put_user() vs put_user by way of commit
> >>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
> >>>
> >>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
> >>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
> >>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
> >>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
> >>> about everything ARM would include this by default?
> >
> > I think the more relevant commit is for 64-bit Arm here, but this does
> > the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
> > eliding access_ok checks in __{get, put}_user").
>
> Ah! Right, this is definitely the important bit, as it makes it
> crystal clear that these are exactly the same thing. The current
> code is:
> #define get_user        __get_user
> #define put_user        __put_user
>
> So, this patch changing from __* to regular versions is a no-op
> on arm side of the house, yea?
>
> > I would think that if we change the __get_user() to get_user()
> > in this driver, the same should be done for the
> > __copy_{from,to}_user(), which similarly skips the access_ok()
> > check but not the PAN/SMAP handling.
>
> Perhaps, thats a good call out. I’d file that under one battle
> at a time. Let’s get get/put user dusted first, then go down
> that road?
>
> > In general, the access_ok()/__get_user()/__copy_from_user()
> > pattern isn't really helpful any more, as Linus already
> > explained. I can't tell from the vhost driver code whether
> > we can just drop the access_ok() here and use the plain
> > get_user()/copy_from_user(), or if it makes sense to move
> > to the newer user_access_begin()/unsafe_get_user()/
> > unsafe_copy_from_user()/user_access_end() and try optimize
> > out a few PAN/SMAP flips in the process.

Right, according to my testing in the past, PAN/SMAP is a killer for
small packet performance (PPS).

>
> In general, I think there are a few spots where we might be
> able to optimize (vhost_get_vq_desc perhaps?) as that gets
> called quite a bit and IIRC there are at least two flips
> in there that perhaps we could elide to one? An investigation
> for another day I think.

Did you mean trying to read descriptors in a batch, that would be
better and with IN_ORDER it would be even faster as a single (at most
two) copy_from_user() might work (without the need to use
user_access_begin()/user_access_end().

>
> Anyhow, with this info - Jason - is there anything else you
> can think of that we want to double click on?

Nope.

Thanks

>
> Jon


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-26 21:45                       ` Linus Torvalds
@ 2025-11-27  2:58                         ` Jon Kohler
  0 siblings, 0 replies; 27+ messages in thread
From: Jon Kohler @ 2025-11-27  2:58 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Arnd Bergmann, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini



> On Nov 26, 2025, at 4:45 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> On Wed, 26 Nov 2025 at 13:43, Jon Kohler <jon@nutanix.com> wrote:
>> 
>> Linus mentioned he might get into the mix and do a bulk
>> change and kill the whole thing once and for all, so I’m
>> simply trying to help knock an incremental amount of work
>> off the pile in advance of that (and reap some performance
>> benefits at the same time, at least on the x86 side).
> 
> So I'm definitely going to do some bulk conversion at some point, but
> honestly, I'll be a lot happier if most users already self-converted
> before that, and I only end up doing a "convert unmaintained old code
> that nobody really cares about".
> 
>                  Linus

That is fair, thanks Linus. I’ll see if I can pick off a couple
more to start thinning out the pile, and see what sort of trouble
I can get into.

Cheers - Jon

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-27  1:08                   ` Jason Wang
@ 2025-11-27  3:11                     ` Jon Kohler
  2025-11-27  6:31                       ` Michael S. Tsirkin
  2025-11-27  6:32                       ` Michael S. Tsirkin
  0 siblings, 2 replies; 27+ messages in thread
From: Jon Kohler @ 2025-11-27  3:11 UTC (permalink / raw)
  To: Jason Wang
  Cc: Arnd Bergmann, Michael S. Tsirkin, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini



> On Nov 26, 2025, at 8:08 PM, Jason Wang <jasowang@redhat.com> wrote:
> 
> On Thu, Nov 27, 2025 at 3:48 AM Jon Kohler <jon@nutanix.com> wrote:
>> 
>> 
>>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> 
>>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
>>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
>>>>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
>>>>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
>>>>> Same deal goes for __put_user() vs put_user by way of commit
>>>>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
>>>>> 
>>>>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
>>>>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
>>>>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
>>>>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
>>>>> about everything ARM would include this by default?
>>> 
>>> I think the more relevant commit is for 64-bit Arm here, but this does
>>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
>>> eliding access_ok checks in __{get, put}_user").
>> 
>> Ah! Right, this is definitely the important bit, as it makes it
>> crystal clear that these are exactly the same thing. The current
>> code is:
>> #define get_user        __get_user
>> #define put_user        __put_user
>> 
>> So, this patch changing from __* to regular versions is a no-op
>> on arm side of the house, yea?
>> 
>>> I would think that if we change the __get_user() to get_user()
>>> in this driver, the same should be done for the
>>> __copy_{from,to}_user(), which similarly skips the access_ok()
>>> check but not the PAN/SMAP handling.
>> 
>> Perhaps, thats a good call out. I’d file that under one battle
>> at a time. Let’s get get/put user dusted first, then go down
>> that road?
>> 
>>> In general, the access_ok()/__get_user()/__copy_from_user()
>>> pattern isn't really helpful any more, as Linus already
>>> explained. I can't tell from the vhost driver code whether
>>> we can just drop the access_ok() here and use the plain
>>> get_user()/copy_from_user(), or if it makes sense to move
>>> to the newer user_access_begin()/unsafe_get_user()/
>>> unsafe_copy_from_user()/user_access_end() and try optimize
>>> out a few PAN/SMAP flips in the process.
> 
> Right, according to my testing in the past, PAN/SMAP is a killer for
> small packet performance (PPS).

For sure, every little bit helps along that path

> 
>> 
>> In general, I think there are a few spots where we might be
>> able to optimize (vhost_get_vq_desc perhaps?) as that gets
>> called quite a bit and IIRC there are at least two flips
>> in there that perhaps we could elide to one? An investigation
>> for another day I think.
> 
> Did you mean trying to read descriptors in a batch, that would be
> better and with IN_ORDER it would be even faster as a single (at most
> two) copy_from_user() might work (without the need to use
> user_access_begin()/user_access_end().

Yep. I haven’t fully thought through it, just a drive-by idea
from looking at code for the recent work I’ve been doing, just
scratching my head thinking there *must* be something we can do
better there.

Basically on the get rx/tx bufs path as well as the
vhost_add_used_and_signal_n path, I think we could cluster together
some of the get/put users and copy to/from’s. Would take some
massaging, but I think there is something there.

>> 
>> Anyhow, with this info - Jason - is there anything else you
>> can think of that we want to double click on?
> 
> Nope.
> 
> Thanks

Ok thanks. Perhaps we can land this in next and let it soak in,
though, knock on wood, I don’t think there will be fallout
(famous last words!) ?


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-27  3:11                     ` Jon Kohler
@ 2025-11-27  6:31                       ` Michael S. Tsirkin
  2025-11-27  6:32                       ` Michael S. Tsirkin
  1 sibling, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2025-11-27  6:31 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Jason Wang, Arnd Bergmann, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini

On Thu, Nov 27, 2025 at 03:11:57AM +0000, Jon Kohler wrote:
> 
> 
> > On Nov 26, 2025, at 8:08 PM, Jason Wang <jasowang@redhat.com> wrote:
> > 
> > On Thu, Nov 27, 2025 at 3:48 AM Jon Kohler <jon@nutanix.com> wrote:
> >> 
> >> 
> >>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >>> 
> >>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
> >>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
> >>>>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
> >>>>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
> >>>>> Same deal goes for __put_user() vs put_user by way of commit
> >>>>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
> >>>>> 
> >>>>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
> >>>>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
> >>>>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
> >>>>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
> >>>>> about everything ARM would include this by default?
> >>> 
> >>> I think the more relevant commit is for 64-bit Arm here, but this does
> >>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
> >>> eliding access_ok checks in __{get, put}_user").
> >> 
> >> Ah! Right, this is definitely the important bit, as it makes it
> >> crystal clear that these are exactly the same thing. The current
> >> code is:
> >> #define get_user        __get_user
> >> #define put_user        __put_user
> >> 
> >> So, this patch changing from __* to regular versions is a no-op
> >> on arm side of the house, yea?
> >> 
> >>> I would think that if we change the __get_user() to get_user()
> >>> in this driver, the same should be done for the
> >>> __copy_{from,to}_user(), which similarly skips the access_ok()
> >>> check but not the PAN/SMAP handling.
> >> 
> >> Perhaps, thats a good call out. I’d file that under one battle
> >> at a time. Let’s get get/put user dusted first, then go down
> >> that road?
> >> 
> >>> In general, the access_ok()/__get_user()/__copy_from_user()
> >>> pattern isn't really helpful any more, as Linus already
> >>> explained. I can't tell from the vhost driver code whether
> >>> we can just drop the access_ok() here and use the plain
> >>> get_user()/copy_from_user(), or if it makes sense to move
> >>> to the newer user_access_begin()/unsafe_get_user()/
> >>> unsafe_copy_from_user()/user_access_end() and try optimize
> >>> out a few PAN/SMAP flips in the process.
> > 
> > Right, according to my testing in the past, PAN/SMAP is a killer for
> > small packet performance (PPS).
> 
> For sure, every little bit helps along that path
> 
> > 
> >> 
> >> In general, I think there are a few spots where we might be
> >> able to optimize (vhost_get_vq_desc perhaps?) as that gets
> >> called quite a bit and IIRC there are at least two flips
> >> in there that perhaps we could elide to one? An investigation
> >> for another day I think.
> > 
> > Did you mean trying to read descriptors in a batch, that would be
> > better and with IN_ORDER it would be even faster as a single (at most
> > two) copy_from_user() might work (without the need to use
> > user_access_begin()/user_access_end().
> 
> Yep. I haven’t fully thought through it, just a drive-by idea
> from looking at code for the recent work I’ve been doing, just
> scratching my head thinking there *must* be something we can do
> better there.
> 
> Basically on the get rx/tx bufs path as well as the
> vhost_add_used_and_signal_n path, I think we could cluster together
> some of the get/put users and copy to/from’s. Would take some
> massaging, but I think there is something there.
> 
> >> 
> >> Anyhow, with this info - Jason - is there anything else you
> >> can think of that we want to double click on?
> > 
> > Nope.
> > 
> > Thanks
> 
> Ok thanks. Perhaps we can land this in next and let it soak in,
> though, knock on wood, I don’t think there will be fallout
> (famous last words!) ?

Yea I'll put this in linux-next and we'll see what happens.


-- 
MST


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-27  3:11                     ` Jon Kohler
  2025-11-27  6:31                       ` Michael S. Tsirkin
@ 2025-11-27  6:32                       ` Michael S. Tsirkin
  2025-12-02 16:54                         ` Jon Kohler
  1 sibling, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2025-11-27  6:32 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Jason Wang, Arnd Bergmann, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini

On Thu, Nov 27, 2025 at 03:11:57AM +0000, Jon Kohler wrote:
> 
> 
> > On Nov 26, 2025, at 8:08 PM, Jason Wang <jasowang@redhat.com> wrote:
> > 
> > On Thu, Nov 27, 2025 at 3:48 AM Jon Kohler <jon@nutanix.com> wrote:
> >> 
> >> 
> >>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >>> 
> >>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
> >>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
> >>>>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
> >>>>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
> >>>>> Same deal goes for __put_user() vs put_user by way of commit
> >>>>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
> >>>>> 
> >>>>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
> >>>>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
> >>>>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
> >>>>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
> >>>>> about everything ARM would include this by default?
> >>> 
> >>> I think the more relevant commit is for 64-bit Arm here, but this does
> >>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
> >>> eliding access_ok checks in __{get, put}_user").
> >> 
> >> Ah! Right, this is definitely the important bit, as it makes it
> >> crystal clear that these are exactly the same thing. The current
> >> code is:
> >> #define get_user        __get_user
> >> #define put_user        __put_user
> >> 
> >> So, this patch changing from __* to regular versions is a no-op
> >> on arm side of the house, yea?
> >> 
> >>> I would think that if we change the __get_user() to get_user()
> >>> in this driver, the same should be done for the
> >>> __copy_{from,to}_user(), which similarly skips the access_ok()
> >>> check but not the PAN/SMAP handling.
> >> 
> >> Perhaps, thats a good call out. I’d file that under one battle
> >> at a time. Let’s get get/put user dusted first, then go down
> >> that road?
> >> 
> >>> In general, the access_ok()/__get_user()/__copy_from_user()
> >>> pattern isn't really helpful any more, as Linus already
> >>> explained. I can't tell from the vhost driver code whether
> >>> we can just drop the access_ok() here and use the plain
> >>> get_user()/copy_from_user(), or if it makes sense to move
> >>> to the newer user_access_begin()/unsafe_get_user()/
> >>> unsafe_copy_from_user()/user_access_end() and try optimize
> >>> out a few PAN/SMAP flips in the process.
> > 
> > Right, according to my testing in the past, PAN/SMAP is a killer for
> > small packet performance (PPS).
> 
> For sure, every little bit helps along that path
> 
> > 
> >> 
> >> In general, I think there are a few spots where we might be
> >> able to optimize (vhost_get_vq_desc perhaps?) as that gets
> >> called quite a bit and IIRC there are at least two flips
> >> in there that perhaps we could elide to one? An investigation
> >> for another day I think.
> > 
> > Did you mean trying to read descriptors in a batch, that would be
> > better and with IN_ORDER it would be even faster as a single (at most
> > two) copy_from_user() might work (without the need to use
> > user_access_begin()/user_access_end().
> 
> Yep. I haven’t fully thought through it, just a drive-by idea
> from looking at code for the recent work I’ve been doing, just
> scratching my head thinking there *must* be something we can do
> better there.
> 
> Basically on the get rx/tx bufs path as well as the
> vhost_add_used_and_signal_n path, I think we could cluster together
> some of the get/put users and copy to/from’s. Would take some
> massaging, but I think there is something there.
> 
> >> 
> >> Anyhow, with this info - Jason - is there anything else you
> >> can think of that we want to double click on?
> > 
> > Nope.
> > 
> > Thanks
> 
> Ok thanks. Perhaps we can land this in next and let it soak in,
> though, knock on wood, I don’t think there will be fallout
> (famous last words!) ?
> 


To clairify, I think vhost tree is better to put this
in next than net-next, both because it's purely core vhost
and because unlike net-next vhost rebases so it is easy to
just drop the patch if there are issues.
I'll put it there.

-- 
MST


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH net-next] vhost: use "checked" versions of get_user() and put_user()
  2025-11-27  6:32                       ` Michael S. Tsirkin
@ 2025-12-02 16:54                         ` Jon Kohler
  0 siblings, 0 replies; 27+ messages in thread
From: Jon Kohler @ 2025-12-02 16:54 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jason Wang, Arnd Bergmann, Eugenio Pérez,
	kvm@vger.kernel.org, virtualization@lists.linux.dev, Netdev,
	linux-kernel@vger.kernel.org, Linus Torvalds, Borislav Petkov,
	Sean Christopherson, linux-arm-kernel@lists.infradead.org,
	Russell King, Catalin Marinas, Will Deacon, Krzysztof Kozlowski,
	Alexandre Belloni, Linus Walleij, Drew Fustini



> On Nov 27, 2025, at 1:32 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> On Thu, Nov 27, 2025 at 03:11:57AM +0000, Jon Kohler wrote:
>> 
>> 
>>> On Nov 26, 2025, at 8:08 PM, Jason Wang <jasowang@redhat.com> wrote:
>>> 
>>> On Thu, Nov 27, 2025 at 3:48 AM Jon Kohler <jon@nutanix.com> wrote:
>>>> 
>>>> 
>>>>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>>> 
>>>>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
>>>>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@nutanix.com> wrote:
>>>>>>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@redhat.com> wrote:
>>>>>>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@nutanix.com> wrote:
>>>>>>> Same deal goes for __put_user() vs put_user by way of commit
>>>>>>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
>>>>>>> 
>>>>>>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
>>>>>>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
>>>>>>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
>>>>>>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
>>>>>>> about everything ARM would include this by default?
>>>>> 
>>>>> I think the more relevant commit is for 64-bit Arm here, but this does
>>>>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
>>>>> eliding access_ok checks in __{get, put}_user").
>>>> 
>>>> Ah! Right, this is definitely the important bit, as it makes it
>>>> crystal clear that these are exactly the same thing. The current
>>>> code is:
>>>> #define get_user        __get_user
>>>> #define put_user        __put_user
>>>> 
>>>> So, this patch changing from __* to regular versions is a no-op
>>>> on arm side of the house, yea?
>>>> 
>>>>> I would think that if we change the __get_user() to get_user()
>>>>> in this driver, the same should be done for the
>>>>> __copy_{from,to}_user(), which similarly skips the access_ok()
>>>>> check but not the PAN/SMAP handling.
>>>> 
>>>> Perhaps, thats a good call out. I’d file that under one battle
>>>> at a time. Let’s get get/put user dusted first, then go down
>>>> that road?
>>>> 
>>>>> In general, the access_ok()/__get_user()/__copy_from_user()
>>>>> pattern isn't really helpful any more, as Linus already
>>>>> explained. I can't tell from the vhost driver code whether
>>>>> we can just drop the access_ok() here and use the plain
>>>>> get_user()/copy_from_user(), or if it makes sense to move
>>>>> to the newer user_access_begin()/unsafe_get_user()/
>>>>> unsafe_copy_from_user()/user_access_end() and try optimize
>>>>> out a few PAN/SMAP flips in the process.
>>> 
>>> Right, according to my testing in the past, PAN/SMAP is a killer for
>>> small packet performance (PPS).
>> 
>> For sure, every little bit helps along that path
>> 
>>> 
>>>> 
>>>> In general, I think there are a few spots where we might be
>>>> able to optimize (vhost_get_vq_desc perhaps?) as that gets
>>>> called quite a bit and IIRC there are at least two flips
>>>> in there that perhaps we could elide to one? An investigation
>>>> for another day I think.
>>> 
>>> Did you mean trying to read descriptors in a batch, that would be
>>> better and with IN_ORDER it would be even faster as a single (at most
>>> two) copy_from_user() might work (without the need to use
>>> user_access_begin()/user_access_end().
>> 
>> Yep. I haven’t fully thought through it, just a drive-by idea
>> from looking at code for the recent work I’ve been doing, just
>> scratching my head thinking there *must* be something we can do
>> better there.
>> 
>> Basically on the get rx/tx bufs path as well as the
>> vhost_add_used_and_signal_n path, I think we could cluster together
>> some of the get/put users and copy to/from’s. Would take some
>> massaging, but I think there is something there.
>> 
>>>> 
>>>> Anyhow, with this info - Jason - is there anything else you
>>>> can think of that we want to double click on?
>>> 
>>> Nope.
>>> 
>>> Thanks
>> 
>> Ok thanks. Perhaps we can land this in next and let it soak in,
>> though, knock on wood, I don’t think there will be fallout
>> (famous last words!) ?
>> 
> 
> 
> To clairify, I think vhost tree is better to put this
> in next than net-next, both because it's purely core vhost
> and because unlike net-next vhost rebases so it is easy to
> just drop the patch if there are issues.
> I'll put it there.
> 
> -- 
> MST
> 

Ok cool, thank you!

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2025-12-02 16:55 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-13  0:55 [PATCH net-next] vhost: use "checked" versions of get_user() and put_user() Jon Kohler
2025-11-13  1:09 ` Jason Wang
2025-11-14 14:53   ` Jon Kohler
2025-11-14 17:48     ` Linus Torvalds
2025-11-14 19:08       ` David Laight
2025-11-14 20:48         ` Linus Torvalds
2025-11-14 21:38           ` David Laight
2025-11-17  4:32     ` Jason Wang
2025-11-17 17:34       ` Jon Kohler
2025-11-20  1:57         ` Jason Wang
2025-11-25 19:45           ` Jon Kohler
2025-11-26  6:04             ` Jason Wang
2025-11-26 10:25               ` Arnd Bergmann
2025-11-26 19:47                 ` Jon Kohler
2025-11-26 19:58                   ` Arnd Bergmann
2025-11-26 21:42                     ` Jon Kohler
2025-11-26 21:45                       ` Linus Torvalds
2025-11-27  2:58                         ` Jon Kohler
2025-11-27  1:08                   ` Jason Wang
2025-11-27  3:11                     ` Jon Kohler
2025-11-27  6:31                       ` Michael S. Tsirkin
2025-11-27  6:32                       ` Michael S. Tsirkin
2025-12-02 16:54                         ` Jon Kohler
2025-11-14 18:54 ` David Laight
2025-11-14 19:30   ` Jon Kohler
2025-11-14 20:32     ` David Laight
2025-11-16  6:32     ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).