Re: [PATCH 2/2] uaccess: minimize INLINE_COPY_USER-related ifdefery

[Christophe Leroy (CS GROUP)]

Le 24/04/2026 à 21:50, Yury Norov a écrit :
> On Thu, Mar 26, 2026 at 03:15:09PM +0100, Christophe Leroy (CS GROUP) wrote:
> > Le 25/03/2026 à 17:33, Yury Norov a écrit :
> > > Now that we've got the same knob selecting inline vs outline copy_to_user()
> > > and copy_from_user(), we can simplify the corresponding logic in the
> > > uaccess.h.
> >
> > I think we should get rid of ifdefery completly, see below. And with comment
> > to previous patch,
> >
> >      __is_defined(INLINE_COPY_TO_USER)
> >
> > becomes
> >
> >      !IS_ENABLED(CONFIG_ARCH_WANT_OUTLINE_USER_COPY)
>
> It's out of the scope of the series. Feel free to submit a follow-up.

Why ? Seting an option through a const macro in a header is historical. 
Nowadays this is done with Kconfig. If you are going to change 
INLINE_COPY_TO_USER/INLINE_COPY_FROM_USER to a new symbol, why remain 
with historical/deprecated practice ?

> > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> > index 327f967a24b8..02a05dd61a77 100644
> > --- a/include/linux/uaccess.h
> > +++ b/include/linux/uaccess.h
> > @@ -190,10 +190,9 @@ _inline_copy_from_user(void *to, const void __user
> > *from, unsigned long n)
> >   	memset(to + (n - res), 0, res);
> >   	return res;
> >   }
> > -#ifndef INLINE_COPY_FROM_USER
> > +
> >   extern __must_check unsigned long
> >   _copy_from_user(void *, const void __user *, unsigned long);
> > -#endif
> >
> >   static inline __must_check unsigned long
> >   _inline_copy_to_user(void __user *to, const void *from, unsigned long n)
> > @@ -207,21 +206,19 @@ _inline_copy_to_user(void __user *to, const void
> > *from, unsigned long n)
> >   	}
> >   	return n;
> >   }
> > -#ifndef INLINE_COPY_TO_USER
> > +
> >   extern __must_check unsigned long
> >   _copy_to_user(void __user *, const void *, unsigned long);
> > -#endif
>
> This declares a function that may have no implementation. It's wrong.

This is common practice in the kernel, what's wrong with that ?

> >   static __always_inline unsigned long __must_check
> >   copy_from_user_common(void *to, const void __user *from, unsigned long n,
> > bool partial)
> >   {
> >   	if (!check_copy_size(to, n, false))
> >   		return n;
> > -#ifdef INLINE_COPY_FROM_USER
> > -	return _inline_copy_from_user(to, from, n);
> > -#else
> > -	return _copy_from_user(to, from, n);
> > -#endif
> > +	if (__is_defined(INLINE_COPY_FROM_USER))
> > +		return _inline_copy_from_user(to, from, n);
> > +	else
> > +		return _copy_from_user(to, from, n);
>
> To me, this is just another form of ifdefery. I prefer to minimize the
> number of IS_DEFINED() blocks, just as #ifdef. But it's maybe just me.

Have you read 
https://docs.kernel.org/process/coding-style.html#conditional-compilation ?

> Let's merge INLINE_COPY_USER series, and then see how you follow up
> would look?

Does this series have a real added value ? Is the churn in the arch's 
asm/uaccess.h worth it ? Why don't go immediatly to a Kconfig option 
which is the current practice nowadays ?

Christophe