Re: [PATCH] Don't test nanoseconds for non-LFS interface in io/tst-stat.c

[Stefan Liebler <stli@linux.ibm.com>]

On 16/03/2021 20:59, Adhemerval Zanella wrote:
> 
> 
> On 16/03/2021 13:30, Stefan Liebler wrote:
>> On 16/03/2021 15:24, Adhemerval Zanella wrote:
>>>
>>>
>>> On 16/03/2021 10:56, Stefan Liebler wrote:
>>>> Both new tests io/tst-stat and io/tst-stat-lfs (_FILE_OFFSET_BITS=64)
>>>> are comparing the nanosecond fields with the statx result.  Unfortunately
>>>> e.g. on s390(31bit) those fields are always zero if old KABI with non-LFS
>>>> support is used.  With _FILE_OFFSET_BITS=64 stat is using statx internally.
>>>
>>> This might also happens for LFS interface if statx is not supported by the
>>> kernel, since the LFS call will fall back to the use the stat syscall that
>>> has this issue.
>>>
>>> Maybe it would be better to make it an internal tests and add a flag
>>> somewhere to just disable it for s390-32. 
>>>
>> gcc is setting the macros __s390x__ and __s390__ on s390x(64bit), but
>> only __s390__ on s390(31bit). Thus I can detect s390(31bit) at
>> compile-time via "#if" and disable the nanoseconds checks on s390(31bit)
>> at all and run it on all other cases. Then I don't need to make the test
>> an internal test and don't need special flags. If this is okay for you,
>> I will prepare a further patch (also with a different subject).
> 
> I think it would better to add such logic on libsupport instead directly
> on the test itself, similar to what support_path_support_time64 does. 
> Maybe something like:
> 
>   bool
>   support_stat_nanoseconds (void)
>   {
>     /* s390 stat64 compat symbol does not support nanoseconds resolution
>        and it used on non-LFS [f,l]stat[at] implementations.  */
>   #if defined __linux__ && !defined __s390x__ && defined __s390__
>     return false;
>   #else
>     return true;
>   }
> 
> Another possibility is if you want to fix it for s390 is to call
> statx on Linux non-LFS fstatat call (sysdeps/unix/sysv/linux/fstatat.c)
> which should fix stat, lstat, and fstat as well.  You will need to
> add a stat to statx conversion at sysdeps/unix/sysv/linux/statx_cp.c
> which would need handle EOVERFLOW on st_ino, st_size, and st_blocks.
> 

I'm fine with just disabling the nanoseconds test on s390(31bit). I've
also talked to the kernel guys. They won't fix the compat layer in this
case.

Please have a look at the new patch
"[PATCH] S390: Don't test nanoseconds in io/tst-stat.c"
https://sourceware.org/pipermail/libc-alpha/2021-March/124063.html

Thanks,
Stefan