Re: [PATCH 1/2] vhost: set dirty log bits without pinning pages on x86

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Nov 12, 2026 at 05:10:10AM -0500, Michael S. Tsirkin wrote:
> On Thu, Nov 12, 2026 at 04:56:30PM +0800, zhaoguohan@kylinos.cn wrote:
> > From: Khan Zhao <zhaoguohan@kylinos.cn>
> > 
> > set_bit_to_user() pins the log page with pin_user_pages_fast(), maps it
> > with kmap_atomic(), sets one bit and then unpins the page and marks it
> > dirty. This runs for every bit set in the dirty log, e.g. on each used
> > ring update while logging is enabled for live migration.
> > 
> > log_write() always passes a byte address and a bit number in [0, 7], so
> > the update can be done as a byte-sized cmpxchg on the user address. On
> > x86, try that first using unsafe_try_cmpxchg_user() inside a
> > user_access_begin()/user_access_end() section with page faults disabled.
> > If the access faults (page not present, read-only or needing COW), or
> > the cmpxchg loses to concurrent updates of the same byte four times in
> > a row, fall back to the existing pin/kmap/set_bit path, which can fault
> > the page in.
> > 
> > unsafe_try_cmpxchg_user() is only implemented on x86, so other
> > architectures keep using the existing path.
> > 
> > On an x86_64 KVM guest (i7-12800HX host), a synthetic used-ring
> > workload with logging enabled went from 243.60 to 171.11 ns per
> > descriptor, averaged over five alternating runs. This is the
> > end-to-end cost per descriptor, not live migration performance.
> > 
> > Signed-off-by: Khan Zhao <zhaoguohan@kylinos.cn>
> > ---
> >  drivers/vhost/vhost.c | 50 ++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 45 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index e29f154706da..9c3045504709 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2527,11 +2527,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> >  }
> >  EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
> >  
> > -/* TODO: This is really inefficient.  We need something like get_user()
> > - * (instruction directly accesses the data, with an exception table entry
> > - * returning -EFAULT). See Documentation/arch/x86/exception-tables.rst.
> > - */
> > -static int set_bit_to_user(int nr, void __user *addr)
> > +static int set_bit_to_user_slow(int nr, void __user *addr)
> >  {
> >  	unsigned long log = (unsigned long)addr;
> >  	struct page *page;
> > @@ -2550,6 +2546,50 @@ static int set_bit_to_user(int nr, void __user *addr)
> >  	return 0;
> >  }
> >  
> > +#ifdef CONFIG_X86
> 
> Wait this is x86 specific anyway? So let's just implement it
> properly, why do we need all these hacks?
> 
> I run a quick test on this in a VM, feel free to reuse:
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 

sorry fat fingers. wrong file. this one seems to work.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 3a0dd3c2b233e..ba1e7c3f83c8e 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -594,6 +594,24 @@ _label:									\
 	__ret;								\
 							})
 
+/* set_bit() on a byte of user memory; returns -EFAULT on fault. */
+#define set_bit_user(nr, addr)					\
+({									\
+	int __err = 0;							\
+	__uaccess_begin();						\
+	asm volatile("\n"						\
+		     "1: " LOCK_PREFIX "orb %[mask], %[ptr]\n"		\
+		     "2:\n"						\
+		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG,	\
+					   %[errout])			\
+		     : [errout] "=r" (__err),				\
+		       [ptr] "+m" (*(u8 __user *)(addr))		\
+		     : [mask] "q" ((unsigned char)BIT(nr))		\
+		     : "memory", "cc");				\
+	__uaccess_end();						\
+	__err ? -EFAULT : 0;						\
+})
+
 /*
  * We want the unsafe accessors to always be inlined and use
  * the error labels - thus the macro games.