[PATCH] LoongArch: Add fstat64 and fstatat64.

[caiyinyu]

v3 links 
https://sourceware.org/pipermail/libc-alpha/2024-September/159902.html

在 2024/9/12 下午8:45, caiyinyu 写道:
>
> 在 2024/9/12 下午6:52, Xi Ruoyao 写道:
>> On Thu, 2024-09-12 at 17:55 +0800, caiyinyu wrote:
>>> I understand your point, but we cannot guarantee that downstream 
>>> vendors
>>> will
>>> clearly and correctly use --enable-kernel=6.10.6 to build packages,
>>> especially
>>> when there are both kernel packages that support and do not support the
>>> 79 and
>>> 80 system calls, as well as glibc packages that use and do not use the
>>> 79 and
>>> 80 system calls both in the repo. We need the dynamic probing
>>> implementation as a transition.
>> Then they should build Glibc with --enable-
>> kernel=$(THE_MINIMAL_KERNEL_VERSION_THEY_PROVIDE_A_PACKAGE). That's why
>> distros like Ubuntu is still using --enable-kernel=3.2 today for
>> x86_64...
>
>
> This really puts us in a difficult position.
> I don't oppose the idea of "Then they should build Glibc with
> --enable-kernel=$(THE_MINIMAL_KERNEL_VERSION_THEY_PROVIDE_A_PACKAGE)." 
> However,
> as I mentioned before, we can't guarantee that downstream vendors will 
> always
> compile it correctly in this way, so I still stand by my approach.
>
> I've submitted a new patch (v3) that fixes several issues:
>
> 1. Removed !defined(__loongarch_lp64) since we currently only have a 
> 64-bit
> implementation.
> 2. __LINUX_KERNEL_VERSION is now used to determine whether to
> apply the dynamic probing solution or the statx-only solution. This way,
> regardless of how the user configures it, the compiled glibc will be 
> able to
> run on all kernels.
>
> v1 links: 
> https://sourceware.org/pipermail/libc-alpha/2024-September/159862.html
> v2 links: 
> https://sourceware.org/pipermail/libc-alpha/2024-September/159889.html
>
>>
>> And I guess I failed to explain the situation.  They don't need --
>> enable-kernel=6.11 to get the gain of the kernel commit
>> 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5.
>>
>> To get a ~50% gain, they just need to upgrade the kernel to a version
>> with the commit 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5.  They don't
>> even need to rebuild Glibc: the optimize is solely in the kernel.
>>
>> And if they want other ~10%, they can apply
>> https://sourceware.org/pipermail/libc-alpha/2024-August/159499.html to
>> Glibc.  This patch has runtime detection, so they still don't need to
>> use --enable-kernel.
>>
>> So if we can backport 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5 to 6.1,
>> 6.6, and 6.10, the performance difference between the patched statx and
>> fstat/newfstatat won't be 60%, but only 3%.  So do we really want
>> hundreds lines of code for a 3% improvement?
>>
>> I.e. if we cannot backport 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5 I'm
>> OK with your proposal for a 60% improvement, but if we can backport
>> 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5 the proposal will only create a
>> 3% improvement and IMHO it won't be worthy.
>>
>> (Also if the distros don't want to upgrade the kernel to 6.11, they
>> likely don't want to upgrade their Glibc to 2.41 anyway.  So IMO it's a
>> good idea to backport 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5 then they
>> can simply get a ~50% improvement by upgrading the kernel to the latest
>> 6.1 or 6.6 LTS point release, w/o upgrading Glibc).
>>
>>> 在 2024/9/12 下午4:37, Xi Ruoyao 写道:
>>>> On Thu, 2024-09-12 at 16:17 +0800, Miao Wang wrote:
>>>>>> 2024年9月12日 16:08，caiyinyu <caiyinyu@loongson.cn> 写道：
>>>>>>
>>>>>>
>>>>>> 在 2024/9/12 上午2:00, Miao Wang 写道:
>>>>>>>> 2024年9月12日 00:50，Miao Wang <shankerwangmiao@gmail.com> 写道：
>>>>>>>>
>>>>>>>>> 2024年9月11日 17:40，caiyinyu <caiyinyu@loongson.cn> 写道：
>>>>>>>>>
>>>>>>>>> In Linux 6.11, the fstat (80) and newfstatat (79) syscalls 
>>>>>>>>> have been
>>>>>>>>> reintroduced. The definitions of these two syscalls have 
>>>>>>>>> already been
>>>>>>>>> backported to version 6.10.6 in the stable tree.
>>>>>>>>>
>>>>>>>>> In this patch, we are adding dynamically probed 
>>>>>>>>> implementations of
>>>>>>>>> fstat64 and fstatat64 specifically for syscalls 79 and 80. 
>>>>>>>>> This ensures
>>>>>>>>> compatibility while maintaining relatively good performance on 
>>>>>>>>> kernels
>>>>>>>>> that both support and do not support syscalls 79 and 80.
>>>>>>>>>
>>>>>>>>> By running an experiment where we invoke fstat64 and fstatat64 
>>>>>>>>> 100
>>>>>>>>> million times, we gathered the following efficiency statistics:
>>>>>>>>>
>>>>>>>>> 1. On kernels that support syscalls 79 and 80 (tested on version
>>>>>>>>>    6.10.6), fstat64 and fstatat64 can directly invoke these 
>>>>>>>>> syscalls
>>>>>>>>>    [1]. The time overhead of our dynamic probing implementation
>>>>>>>>>    increased by 0.5%-2.5% compared to directly calling the 
>>>>>>>>> syscalls.
>>>>>>>>> 2. On kernels that support syscalls 79 and 80 (tested on version
>>>>>>>>>    6.10.6), our dynamically probed implementation reduces the 
>>>>>>>>> time
>>>>>>>>>    overhead by more than 60% compared to directly invoking the 
>>>>>>>>> statx
>>>>>>>>>    (291) syscall.
>>>>>>>>> 3. On kernels that do not support syscalls 79 and 80 (tested 
>>>>>>>>> on version
>>>>>>>>>    6.8.0), fstat64 and fstatat64 fall back to using the statx 
>>>>>>>>> (291)
>>>>>>>>>    syscall (as before). In this case, the overhead of our dynamic
>>>>>>>>>    probing implementation increased by 0.1%-1.3% compared to 
>>>>>>>>> directly
>>>>>>>>>    invoking statx.
>>>>>>>> Hi, I tried to reproduce your test result, by invoking fstat(2) 
>>>>>>>> for 1M times,
>>>>>>>> repeated by 100 times. The test was carried out on 3A6K, using 
>>>>>>>> 6.11-rc7 kernel.
>>>>>>>> The result is:
>>>>>>>>
>>>>>>>> fstat using statx(fd, "", AT_EMPTY_PATH) (current glibc 
>>>>>>>> implementation)
>>>>>>>> mean: 210420528.100000(ns), sigma: 996145.440248(ns)
>>>>>>>>
>>>>>>>> statx(fd, NULL, AT_EMPTY_PATH) (for comparison)
>>>>>>>> mean: 199410620.600000(ns), sigma: 111561.012101(ns)
>>>>>>>>
>>>>>>>> fstat using statx(fd, NULL, AT_EMPTY_PATH) (The implementation 
>>>>>>>> I proposed)
>>>>>>>> mean: 208258640.700000(ns), sigma: 468451.704836(ns)
>>>>>>>>
>>>>>>>> fstat using fstat(fd) (Your patch)
>>>>>>>> mean: 192936673.800000(ns), sigma: 251927.136307(ns)
>>>>>>>>
>>>>>>>> As we can see in the result, the implementation using fstat is 
>>>>>>>> 8.31% faster
>>>>>>>> than the current implementation, instead of "reducing the time 
>>>>>>>> overhead by
>>>>>>>> more than 60%".
>>>>>>> I did another test on 6.10.7, using the current glibc 
>>>>>>> implementation, i.e.
>>>>>>> statx(fd, "", AT_EMPTY_PATH), and got the following result:
>>>>>>>
>>>>>>> mean: 603344203.300000(ns), sigma: 715246.975336(ns)
>>>>>>>
>>>>>>> If we use this as the baseline, we can get the following summary:
>>>>>>>
>>>>>>> fstat(fd): 68.02% less time
>>>>>>> statx(fd, NULL, AT_EMPTY_PATH): 65.48% less time
>>>>>>> statx(fd, "", AT_EMPTY_PATH) (nothing is changed in glibc, only 
>>>>>>> upgrade the
>>>>>>>     kernel to 6.11): 65.12% less time
>>>>>>>
>>>>>>> As a result, the performance gain is similar comparing using 
>>>>>>> fstat(fd) and
>>>>>>> statx(fd, NULL, AT_EMPTY_PATH).
>>>>>>>
>>>>>>>> I prefer introducing dynamic probing of statx(fd, NULL, 
>>>>>>>> AT_EMPTY_PATH), which
>>>>>>>> can benefit all 32-bit platforms relying on statx for 64-bit 
>>>>>>>> timestamps[2], as
>>>>>>>> well as 64-bit loongarch, not only for performance, but also 
>>>>>>>> for seccomp
>>>>>>>> sandboxing. Furthermore, by doing so, we can eliminate the need 
>>>>>>>> of maintaining
>>>>>>>> our own copy of fstat in loongarch.
>>>>>>> To conclude, the question would be whether it is worthy to have 
>>>>>>> a separately
>>>>>>> maintained fstat in loongarch for the 2.54% performance difference.
>>>>>> The key point here is that the dynamic probing solution can run 
>>>>>> directly on
>>>>>> kernels without support for the 79 and 80 system calls (which is 
>>>>>> essential for
>>>>>> us). However, after updating arch-syscall.h, the glibc compiled 
>>>>>> using Miao
>>>>>> plan[1] cannot run on kernels that do not support the 79 and 80 
>>>>>> system calls.
>>>>> Why? In my patch, when the requested kernel compatibility version 
>>>>> is below
>>>>> 6.10.6, __ASSUME_LOONGARCH_NEWSTAT will not be defined, and the 
>>>>> definition
>>>>> of the syscalls __NR_fstat and __NR_newfstatat will be undef-ed. 
>>>>> So the
>>>>> implementation will choose to use statx() in the compile time, and 
>>>>> thus able to
>>>>> run on previous kernels.
>>>> Yes this should be correct.
>>>>
>>>> Let me summarize the situation here:
>>>>
>>>> 1. Using statx for fstat and fstatat was stupidly slow.
>>>> 2. After kernel commit 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5, statx
>>>> is already about 40-50% faster.  No user space change is needed to get
>>>> the gain.  Unfortunately this commit isn't backported.
>>>> 3. After kernel commit 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5, we 
>>>> can
>>>> use NULL instead of "" with AT_EMPTY_PATH.  Miao has already 
>>>> submitted a
>>>> patch for it.  And it will be about 10-20% faster than 2.
>>>> 4. After kernel commit 7697a0fe0154468f5df35c23ebd7aa48994c2cdc, we 
>>>> can
>>>> use native fstat and newfstatat calls on LoongArch 64-bit. It's only
>>>> 2.54% faster than 3 (per Miao, I've not measured but it matches my gut
>>>> feeling).
>>>>
>>>> So the pros of 4:
>>>> a. it works on 6.1.x, 6.6.y, and 6.10.z if [xyz] is large enough to 
>>>> get
>>>> the backport.
>>>> b. 2.54% faster.
>>>>
>>>> The con of 4: maintenance burden.
>>>>
>>>> To solve a. we can backport 
>>>> 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5 to
>>>> 6.1.x, 6.6.y, and 6.10.z.  Then to me b. is really not significant
>>>> enough to outweigh the maintenance burden.
>>>>
>>>> So can we attempt to convince Greg KH to backport
>>>> 0ef625bba6fb2bc0c8ed2aab9524fdf423f67dd5 and use Miao's (simple) patch
>>>> in Glibc instead (and live with the 2.54% difference)?
>>>>
>>>> If both of you can acknowledge the plan I'll write to Greg.
>>>>