This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] misc: Set generic pselect as ENOSYS



On 14/11/2019 17:49, Joseph Myers wrote:
> On Thu, 14 Nov 2019, Adhemerval Zanella wrote:
> 
>> The generic pselect implementation has the very specific race condition
>> that motived the creation of the pselect syscall (no atomicity in
>> signal mask set/reset).  Using it as generic implementation is
>> counterproductive  Also currently only microblaze uses it as fallback
>> when used on kernel prior 3.15.
>>
>> This patch moves the generic implementation to a microblaze specific
>> one, sets the generic internal as a ENOSYS, and cleanups the Linux
>> generic implementation.
>>
>> Also, the microblaze generic implementation first try to issue
>> pselect instead of use the fallback (since it is expect that if
>> the microblaze usage does rely on pselect, a sufficient updated
>> kernel will be used).  Microblaze defines __NR_pselect6 for Linux
>> v3.2, although it was only wire-up on v3.15 (and the syscall number
>> is the same as previous defined).
> 
> I'll raise the same issue here I raise whenever someone proposes having 
> fallback code for old kernels without a corresponding __ASSUME_* macro.
> 
> We need a simple and uniform way, when increasing the minimum kernel 
> version, to find all the fallback code that can be removed.  That means 
> __ASSUME_* macros in kernel-features.h based on __LINUX_KERNEL_VERSION.  
> A free-form text comment mentioning "3.15" is no good for that purpose as 
> there is no sensible automated way to find all such comments when 
> increasing the minimum from 3.2 to 4.4 (for example, as the next such 
> increase that seems to make sense in terms of the cleanups it enables).
> 

My issue with __ASSUME_* is it adds build permutations that ideally should
be checked to get a full tests coverage.  It means we should build and test
with all possible --enable-kernel version for every architecture that might 
be affected by a __ASSUME_*.  I tend to see always building the possible
fallback and assuming a newer kernel tends to be a simpler way.

In any case, I don't have a strong opinion on pselect and I will send an
updated version with __ASSUME_PSELECT back.  It should simplify the
time64 support and I will expect that no changed on microblaze version
should be required.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]