Re: fstat(2) penalized by using newfstatat(6, "", buf, AT_EMPTY_PATH)

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 05/09/23 14:45, Linus Torvalds wrote:
> On Tue, 5 Sept 2023 at 10:28, Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>> I think we can still make it in a generic way, stat family would use more
>> syscall (which some filters might complain) but it should be ok:
> 
> Ugh. That patch of yours certainly seems to avoid the kernel overhead
> (where that "fetch a byte from user space just to check it is '\0'" is
> much more expensive than it is in user space), but why does glibc do
> that whole "turn fstat() into fstatat(), and then turn it back again"?
> 
> It seems just stupid. If the user asked for 'fstat()', just give the
> user 'fstat()'.
> 
> Your patch literally adds *complexity*, rather than take it away.
> 
> Is this all just because glibc did its own 'struct stat'
> implementation way back when, and you want to have just one place
> where the stat conversion is done?
> 
> Can't you just special case *that* case instead? It's long been
> irrelevant. Afaik, on 32-bit x86, you just want to use the 'stat64'
> system call family, and on 64-bit x86 you just use the regular 'stat'
> family (yes, called 'newfstatat' for that one system call for internal
> historial reasons), and there is no conversion needed.
> 
> IOW, all those wrappers just for 'struct stat' conversion are pure
> overhead and silly garbage. Why do they exist? And if there is still
> some other reason for them to exist, can't you make that be a
> 'sysdeps' file of its own, and not penalize all the sane cases with
> 'copy stat structures around'?

The old glibc stat code was a complete mess that was added prior symbol
versioning, with multiple implementations, along with a static linkage
that defines the current version (there is no need to describe it
further). The consolidation also provide a much simpler way to support
y2038.

On x86_64 and some other 64 bits ABIs where glibc exported struct stat 
is  the same as the kernel, there is no conversion whatsoever.  It is 
only required on some ABI where due historical mistakes the exported 
user struct does not match kernel (sparc64, mips64) and on 32 bits 
where the no-LFS support is just routed to LFS interface to simplify
things.

So the fstat to fstatat approach is a way to simplify the code and put 
all the required syscall logic only at fstatat implementation.  By adding 
the 'special' case back on fstat.c it would require to replicate all this
logic plus when to use statx instead (FSTATAT_USE_STATX), which is far 
from ideal. It is doable, but not really simpler...