Re: [PATCH v3] eventpoll: Convert epoll_put_uevent() to scoped user access

[David Laight <david.laight.linux@gmail.com>]

On Sun, 8 Mar 2026 06:19:13 +0100
Eric Dumazet <edumazet@google.com> wrote:

> On Sun, Mar 8, 2026 at 12:45 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Sat, 7 Mar 2026 at 15:33, Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:  
> > >
> > > Oh well. This makes one little piece of the kernel better, even if
> > > other horrors lurk everywhere...  
> >
> > Side note: if you want to improve code generation just a *tiny* bit
> > more, you should make that epoll_put_uevent() return a success value
> > instead ot the updated pointer value.
> >
> > Because right now the caller does this:
> >
> >                 events = epoll_put_uevent(revents, epi->event.data, events);
> >                 if (!events) {
> >
> > and that "if (!events)" test triggers even for the success case,
> > because the compiler doesn't know whether the "uevent+1" might
> > overflow (and we really *do* want to make sure the compiler doesn't
> > remove overflow checks even if it causes these kinds of issues).
> >
> > If epoll_put_uevent() just returns a success/fail marker (or 0/-EFAULT
> > or whatever), we wouldn't have that issue.
> >
> > HOWEVER - the same magical ARM case would need to be dealt with - the
> > real size of the user space notion of "struct epoll_event" isn't
> > actually known to generic code, because it ends up depending on
> > architecture-specific packing issues.
> >
> > So you'd have to pass in 'events' by reference and epoll_put_uevent()
> > would still update it.
> >
> > Probably not worth it - it would mainly get rid of a perfectly
> > predicted conditional branch, and might possibly help a tiny bit with
> > register liveness analysis. But it's unlikely to have any really
> > visible effects - it's not like the clac/stac overhead.
> >
> > But since I looked at the generated code, I thought I'd mention it.
> >
> >                   Linus  
> 
> Thanks Linus
> 
> BTW, not really a bug, but we could remove these extra semicolons
> added in af4e9ef3d784 ("uaccess: Fix scoped_user_read_access() for
> 'pointer to const'")
> 
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 809e4f7dfdbd4d4426ef38457ae108f119cb1bee..cdce14d864d6b71ea87396aee35acd6c9f188ae0
> 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -654,15 +654,15 @@ static inline void user_access_restore(unsigned
> long flags) { }
>  static __always_inline void __scoped_user_read_access_end(const void *p)
>  {
>         user_read_access_end();
> -};
> +}
>  static __always_inline void __scoped_user_write_access_end(const void *p)
>  {
>         user_write_access_end();
> -};
> +}
>  static __always_inline void __scoped_user_rw_access_end(const void *p)
>  {
>         user_access_end();
> -};
> +}

I was clearly writing those too quickly...

	David

> 
>  /**
>   * __scoped_user_access_begin - Start a scoped user access
>