[PATCH v3 2/2] linux: Add linux statx(fd, NULL, AT_EMPTY_PATH) support

Adhemerval Zanella Netto adhemerval.zanella@linaro.org
Wed Aug 21 14:30:52 GMT 2024



On 21/08/24 10:03, Miao Wang via B4 Relay wrote:
> From: Miao Wang <shankerwangmiao@gmail.com>
> 
> Linux supports passing NULL instead of an empty string as the second
> parameter when AT_EMPTY_PATH is set in the flags, starting from 6.11,
> which brings a performance gain since it is much more efficient to
> detect a NULL parameter than to detect an empty string in the kernel.
> We utilize this feature if statx is used for fstat, when glibc is
> compiled to target kernel versions afterwards, and dynamically probe
> the kernel support of it, when targeting previous versions.
> 
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
>  sysdeps/unix/sysv/linux/fstatat64.c        |  9 +++--
>  sysdeps/unix/sysv/linux/fxstat64.c         |  4 +--
>  sysdeps/unix/sysv/linux/kernel-features.h  |  5 +++
>  sysdeps/unix/sysv/linux/statx_empty_path.h | 53 ++++++++++++++++++++++++++++++
>  4 files changed, 67 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
> index da496177c9..5b5ce7d9b5 100644
> --- a/sysdeps/unix/sysv/linux/fstatat64.c
> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
> @@ -39,6 +39,7 @@ _Static_assert (sizeof (__blkcnt_t) == sizeof (__blkcnt64_t),
>  #endif
>  
>  #if FSTATAT_USE_STATX
> +#include <statx_empty_path.h>
>  
>  static inline int
>  fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf,
> @@ -47,8 +48,12 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf,
>    /* 32-bit kABI with default 64-bit time_t, e.g. arc, riscv32.   Also
>       64-bit time_t support is done through statx syscall.  */
>    struct statx tmp;
> -  int r = INTERNAL_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT | flag,
> -				 STATX_BASIC_STATS, &tmp);
> +  int r;
> +  if ((flag & AT_EMPTY_PATH) && (file == NULL || *file == '\0'))
> +    r = __statx_empty_path (fd, AT_NO_AUTOMOUNT | flag, &tmp);
> +  else
> +    r = INTERNAL_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT | flag,
> +			      STATX_BASIC_STATS, &tmp);
>    if (r != 0)
>      return r;
>  
> diff --git a/sysdeps/unix/sysv/linux/fxstat64.c b/sysdeps/unix/sysv/linux/fxstat64.c
> index 230374cb22..9ed4530ca8 100644
> --- a/sysdeps/unix/sysv/linux/fxstat64.c
> +++ b/sysdeps/unix/sysv/linux/fxstat64.c
> @@ -25,6 +25,7 @@
>  #include <xstatconv.h>
>  #include <statx_cp.h>
>  #include <shlib-compat.h>
> +#include <statx_empty_path.h>

The kernel_stat.h is included multiple now, so you will need to add the
include guards on the arch that does not contains them.  For instance,
on armhf it fails with:

sysdeps/unix/sysv/linux/fstatat64.c

In file included from ../sysdeps/unix/sysv/linux/internal-stat.h:21,
                 from ../sysdeps/unix/sysv/linux/statx_empty_path.h:20,
                 from ../sysdeps/unix/sysv/linux/fstatat64.c:42:
../sysdeps/unix/sysv/linux/arm/kernel_stat.h:2:8: error: redefinition of ‘struct kernel_stat’
    2 | struct kernel_stat
      |        ^~~~~~~~~~~
In file included from ../sysdeps/unix/sysv/linux/internal-stat.h:21,
                 from ../sysdeps/unix/sysv/linux/fstatat64.c:27:
../sysdeps/unix/sysv/linux/arm/kernel_stat.h:2:8: note: originally defined here
    2 | struct kernel_stat
      |        ^~~~~~~~~~~
In file included from ../sysdeps/unix/sysv/linux/statx_empty_path.h:21:
../sysdeps/unix/sysv/linux/arm/kernel_stat.h:2:8: error: redefinition of ‘struct kernel_stat’
    2 | struct kernel_stat
      |        ^~~~~~~~~~~
../sysdeps/unix/sysv/linux/arm/kernel_stat.h:2:8: note: originally defined here
    2 | struct kernel_stat
      |        ^~~~~~~~~~~

>  
>  #if LIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_33)
>  
> @@ -53,8 +54,7 @@ ___fxstat64 (int vers, int fd, struct stat64 *buf)
>  # else
>    /* New 32-bit kABIs with only 64-bit time_t support, e.g. arc, riscv32.  */
>    struct statx tmp;
> -  int r = INLINE_SYSCALL_CALL (statx, fd, "", AT_EMPTY_PATH,
> -			       STATX_BASIC_STATS, &tmp);
> +  int r = __statx_empty_path (fd, AT_EMPTY_PATH, &tmp);
>    if (r == 0)
>      __cp_stat64_statx (buf, &tmp);
>    return r;
> diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
> index a25cf07e9f..78aaf43a82 100644
> --- a/sysdeps/unix/sysv/linux/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/kernel-features.h
> @@ -257,4 +257,9 @@
>  # define __ASSUME_FCHMODAT2 0
>  #endif
>  
> +/* statx(fd, NULL, AT_EMPTY_PATH) was introduced in Linux 6.11. */
> +#if __LINUX_KERNEL_VERSION >= 0x060b00
> +# define __ASSUME_STATX_NULL_PATH 1
> +#endif
> +
>  #endif /* kernel-features.h */
> diff --git a/sysdeps/unix/sysv/linux/statx_empty_path.h b/sysdeps/unix/sysv/linux/statx_empty_path.h
> new file mode 100644
> index 0000000000..7c82311ffc
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/statx_empty_path.h
> @@ -0,0 +1,53 @@
> +/* Linux statx(fd, NULL, AT_EMPTY_PATH) support
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <sys/stat.h>
> +#include <internal-stat.h>
> +#include <kernel_stat.h>
> +#include <sysdep.h>
> +
> +#if FSTATAT_USE_STATX || XSTAT_IS_XSTAT64
> +
> +static inline int
> +__statx_empty_path (int fd, int flag, struct statx *buf)
> +{
> +  flag |= AT_EMPTY_PATH;
> +#ifdef __ASSUME_STATX_NULL_PATH
> +  return INTERNAL_SYSCALL_CALL (statx, fd, NULL, flag, STATX_BASIC_STATS, buf);
> +#else
> +  static int statx_null_path_supported = -1;
> +  int r;
> +  int supported = atomic_load_relaxed (&statx_null_path_supported);
> +  if (supported != 0)
> +    {
> +      r = INTERNAL_SYSCALL_CALL (statx, fd, NULL, flag, STATX_BASIC_STATS, buf);
> +      if (__glibc_likely(supported == 1))
> +	return r;
> +      if (r != -EFAULT)
> +	{
> +	  if (r == 0)
> +            atomic_store_relaxed (&statx_null_path_supported, 1);
> +	  return r;
> +	}
> +      atomic_store_relaxed (&statx_null_path_supported, 0);
> +    }
> +  return INTERNAL_SYSCALL_CALL (statx, fd, "", flag, STATX_BASIC_STATS, buf);
> +#endif
> +}
> +
> +#endif
> 

I would prefer to just add it on internal-stat.h, it already contains some
internal logic for stat functions and I think it should fit it better than
another internal header.ib


More information about the Libc-alpha mailing list