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

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

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