diff mbox series

[v2] xfs_io: add support for atomic write statx fields

Message ID 20241118235255.23133-1-catherine.hoang@oracle.com (mailing list archive)
State New
Headers show
Series [v2] xfs_io: add support for atomic write statx fields | expand

Commit Message

Catherine Hoang Nov. 18, 2024, 11:52 p.m. UTC
Add support for the new atomic_write_unit_min, atomic_write_unit_max, and
atomic_write_segments_max fields in statx for xfs_io. In order to support builds
against old kernel headers, define our own internal statx structs. If the
system's struct statx does not have the required atomic write fields, override
the struct definitions with the internal definitions in statx.h.

Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
 configure.ac          |  1 +
 include/builddefs.in  |  4 ++++
 io/stat.c             |  7 +++++++
 io/statx.h            | 23 ++++++++++++++++++++++-
 m4/package_libcdev.m4 | 20 ++++++++++++++++++++
 5 files changed, 54 insertions(+), 1 deletion(-)

Comments

Darrick J. Wong Nov. 19, 2024, 12:01 a.m. UTC | #1
On Mon, Nov 18, 2024 at 03:52:55PM -0800, Catherine Hoang wrote:
> Add support for the new atomic_write_unit_min, atomic_write_unit_max, and
> atomic_write_segments_max fields in statx for xfs_io. In order to support builds
> against old kernel headers, define our own internal statx structs. If the
> system's struct statx does not have the required atomic write fields, override
> the struct definitions with the internal definitions in statx.h.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  configure.ac          |  1 +
>  include/builddefs.in  |  4 ++++
>  io/stat.c             |  7 +++++++
>  io/statx.h            | 23 ++++++++++++++++++++++-
>  m4/package_libcdev.m4 | 20 ++++++++++++++++++++
>  5 files changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 33b01399..0b1ef3c3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -146,6 +146,7 @@ AC_HAVE_COPY_FILE_RANGE
>  AC_NEED_INTERNAL_FSXATTR
>  AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG
>  AC_NEED_INTERNAL_FSCRYPT_POLICY_V2
> +AC_NEED_INTERNAL_STATX
>  AC_HAVE_GETFSMAP
>  AC_HAVE_MAP_SYNC
>  AC_HAVE_DEVMAPPER
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 1647d2cd..cbc9ab0c 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -96,6 +96,7 @@ HAVE_COPY_FILE_RANGE = @have_copy_file_range@
>  NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
>  NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@
>  NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@
> +NEED_INTERNAL_STATX = @need_internal_statx@
>  HAVE_GETFSMAP = @have_getfsmap@
>  HAVE_MAP_SYNC = @have_map_sync@
>  HAVE_DEVMAPPER = @have_devmapper@
> @@ -130,6 +131,9 @@ endif
>  ifeq ($(NEED_INTERNAL_FSCRYPT_POLICY_V2),yes)
>  PCFLAGS+= -DOVERRIDE_SYSTEM_FSCRYPT_POLICY_V2
>  endif
> +ifeq ($(NEED_INTERNAL_STATX),yes)
> +PCFLAGS+= -DOVERRIDE_SYSTEM_STATX
> +endif
>  ifeq ($(HAVE_GETFSMAP),yes)
>  PCFLAGS+= -DHAVE_GETFSMAP
>  endif
> diff --git a/io/stat.c b/io/stat.c
> index 0f5618f6..7d1c1c93 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -6,6 +6,10 @@
>   * Portions of statx support written by David Howells (dhowells@redhat.com)
>   */
>  
> +#ifdef OVERRIDE_SYSTEM_STATX
> +#define statx sys_statx
> +#endif
> +
>  #include "command.h"
>  #include "input.h"
>  #include "init.h"
> @@ -347,6 +351,9 @@ dump_raw_statx(struct statx *stx)
>  	printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);
>  	printf("stat.dev_major = %u\n", stx->stx_dev_major);
>  	printf("stat.dev_minor = %u\n", stx->stx_dev_minor);
> +	printf("stat.atomic_write_unit_min = %lld\n", (long long)stx->stx_atomic_write_unit_min);
> +	printf("stat.atomic_write_unit_max = %lld\n", (long long)stx->stx_atomic_write_unit_max);
> +	printf("stat.atomic_write_segments_max = %lld\n", (long long)stx->stx_atomic_write_segments_max);
>  	return 0;
>  }
>  
> diff --git a/io/statx.h b/io/statx.h
> index c6625ac4..d151f732 100644
> --- a/io/statx.h
> +++ b/io/statx.h
> @@ -61,6 +61,7 @@ struct statx_timestamp {
>  	__s32	tv_nsec;
>  	__s32	__reserved;
>  };
> +#endif
>  
>  /*
>   * Structures for the extended file attribute retrieval system call
> @@ -99,6 +100,8 @@ struct statx_timestamp {
>   * will have values installed for compatibility purposes so that stat() and
>   * co. can be emulated in userspace.
>   */
> +#if !defined STATX_TYPE || defined OVERRIDE_SYSTEM_STATX

Is there any place where STATX_TYPE is not defined but
OVERRIDE_SYSTEM_STATX will *also* not be defined?

I think the m4 macro you added sets need_internal_statx=yes for old
systems where there's no STATX_TYPE, because there won't be a struct
statx, let alone atomic_write_* fields in it, right?

--D

> +#undef statx
>  struct statx {
>  	/* 0x00 */
>  	__u32	stx_mask;	/* What results were written [uncond] */
> @@ -126,10 +129,23 @@ struct statx {
>  	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
>  	__u32	stx_dev_minor;
>  	/* 0x90 */
> -	__u64	__spare2[14];	/* Spare space for future expansion */
> +	__u64	stx_mnt_id;
> +	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
> +	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
> +	/* 0xa0 */
> +	__u64	stx_subvol;	/* Subvolume identifier */
> +	__u32	stx_atomic_write_unit_min;	/* Min atomic write unit in bytes */
> +	__u32	stx_atomic_write_unit_max;	/* Max atomic write unit in bytes */
> +	/* 0xb0 */
> +	__u32   stx_atomic_write_segments_max;	/* Max atomic write segment count */
> +	__u32   __spare1[1];
> +	/* 0xb8 */
> +	__u64	__spare3[9];	/* Spare space for future expansion */
>  	/* 0x100 */
>  };
> +#endif
>  
> +#ifndef STATX_TYPE
>  /*
>   * Flags to be stx_mask
>   *
> @@ -174,4 +190,9 @@ struct statx {
>  #define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
>  
>  #endif /* STATX_TYPE */
> +
> +#ifndef STATX_WRITE_ATOMIC
> +#define STATX_WRITE_ATOMIC	0x00010000U	/* Want/got atomic_write_* fields */
> +#endif
> +
>  #endif /* XFS_IO_STATX_H */
> diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
> index 6de8b33e..bc8a49a9 100644
> --- a/m4/package_libcdev.m4
> +++ b/m4/package_libcdev.m4
> @@ -98,6 +98,26 @@ AC_DEFUN([AC_NEED_INTERNAL_FSCRYPT_POLICY_V2],
>      AC_SUBST(need_internal_fscrypt_policy_v2)
>    ])
>  
> +#
> +# Check if we need to override the system struct statx with
> +# the internal definition.  This /only/ happens if the system
> +# actually defines struct statx /and/ the system definition
> +# is missing certain fields.
> +#
> +AC_DEFUN([AC_NEED_INTERNAL_STATX],
> +  [ AC_CHECK_TYPE(struct statx,
> +      [
> +        AC_CHECK_MEMBER(struct statx.stx_atomic_write_unit_min,
> +          ,
> +          need_internal_statx=yes,
> +          [#include <linux/stat.h>]
> +        )
> +      ],,
> +      [#include <linux/stat.h>]
> +    )
> +    AC_SUBST(need_internal_statx)
> +  ])
> +
>  #
>  # Check if we have a FS_IOC_GETFSMAP ioctl (Linux)
>  #
> -- 
> 2.34.1
> 
>
Catherine Hoang Nov. 19, 2024, 2:30 a.m. UTC | #2
> On Nov 18, 2024, at 4:01 PM, Darrick J. Wong <djwong@kernel.org> wrote:
> 
> On Mon, Nov 18, 2024 at 03:52:55PM -0800, Catherine Hoang wrote:
>> Add support for the new atomic_write_unit_min, atomic_write_unit_max, and
>> atomic_write_segments_max fields in statx for xfs_io. In order to support builds
>> against old kernel headers, define our own internal statx structs. If the
>> system's struct statx does not have the required atomic write fields, override
>> the struct definitions with the internal definitions in statx.h.
>> 
>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
>> ---
>> configure.ac          |  1 +
>> include/builddefs.in  |  4 ++++
>> io/stat.c             |  7 +++++++
>> io/statx.h            | 23 ++++++++++++++++++++++-
>> m4/package_libcdev.m4 | 20 ++++++++++++++++++++
>> 5 files changed, 54 insertions(+), 1 deletion(-)
>> 
>> diff --git a/configure.ac b/configure.ac
>> index 33b01399..0b1ef3c3 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -146,6 +146,7 @@ AC_HAVE_COPY_FILE_RANGE
>> AC_NEED_INTERNAL_FSXATTR
>> AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG
>> AC_NEED_INTERNAL_FSCRYPT_POLICY_V2
>> +AC_NEED_INTERNAL_STATX
>> AC_HAVE_GETFSMAP
>> AC_HAVE_MAP_SYNC
>> AC_HAVE_DEVMAPPER
>> diff --git a/include/builddefs.in b/include/builddefs.in
>> index 1647d2cd..cbc9ab0c 100644
>> --- a/include/builddefs.in
>> +++ b/include/builddefs.in
>> @@ -96,6 +96,7 @@ HAVE_COPY_FILE_RANGE = @have_copy_file_range@
>> NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
>> NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@
>> NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@
>> +NEED_INTERNAL_STATX = @need_internal_statx@
>> HAVE_GETFSMAP = @have_getfsmap@
>> HAVE_MAP_SYNC = @have_map_sync@
>> HAVE_DEVMAPPER = @have_devmapper@
>> @@ -130,6 +131,9 @@ endif
>> ifeq ($(NEED_INTERNAL_FSCRYPT_POLICY_V2),yes)
>> PCFLAGS+= -DOVERRIDE_SYSTEM_FSCRYPT_POLICY_V2
>> endif
>> +ifeq ($(NEED_INTERNAL_STATX),yes)
>> +PCFLAGS+= -DOVERRIDE_SYSTEM_STATX
>> +endif
>> ifeq ($(HAVE_GETFSMAP),yes)
>> PCFLAGS+= -DHAVE_GETFSMAP
>> endif
>> diff --git a/io/stat.c b/io/stat.c
>> index 0f5618f6..7d1c1c93 100644
>> --- a/io/stat.c
>> +++ b/io/stat.c
>> @@ -6,6 +6,10 @@
>>  * Portions of statx support written by David Howells (dhowells@redhat.com)
>>  */
>> 
>> +#ifdef OVERRIDE_SYSTEM_STATX
>> +#define statx sys_statx
>> +#endif
>> +
>> #include "command.h"
>> #include "input.h"
>> #include "init.h"
>> @@ -347,6 +351,9 @@ dump_raw_statx(struct statx *stx)
>> printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);
>> printf("stat.dev_major = %u\n", stx->stx_dev_major);
>> printf("stat.dev_minor = %u\n", stx->stx_dev_minor);
>> + printf("stat.atomic_write_unit_min = %lld\n", (long long)stx->stx_atomic_write_unit_min);
>> + printf("stat.atomic_write_unit_max = %lld\n", (long long)stx->stx_atomic_write_unit_max);
>> + printf("stat.atomic_write_segments_max = %lld\n", (long long)stx->stx_atomic_write_segments_max);
>> return 0;
>> }
>> 
>> diff --git a/io/statx.h b/io/statx.h
>> index c6625ac4..d151f732 100644
>> --- a/io/statx.h
>> +++ b/io/statx.h
>> @@ -61,6 +61,7 @@ struct statx_timestamp {
>> __s32 tv_nsec;
>> __s32 __reserved;
>> };
>> +#endif
>> 
>> /*
>>  * Structures for the extended file attribute retrieval system call
>> @@ -99,6 +100,8 @@ struct statx_timestamp {
>>  * will have values installed for compatibility purposes so that stat() and
>>  * co. can be emulated in userspace.
>>  */
>> +#if !defined STATX_TYPE || defined OVERRIDE_SYSTEM_STATX
> 
> Is there any place where STATX_TYPE is not defined but
> OVERRIDE_SYSTEM_STATX will *also* not be defined?

I don’t think so. So I guess this could just be

#ifdef OVERRIDE_SYSTEM_STATX

Does that seem right?
> 
> I think the m4 macro you added sets need_internal_statx=yes for old
> systems where there's no STATX_TYPE, because there won't be a struct
> statx, let alone atomic_write_* fields in it, right?
> 
> --D
> 
>> +#undef statx
>> struct statx {
>> /* 0x00 */
>> __u32 stx_mask; /* What results were written [uncond] */
>> @@ -126,10 +129,23 @@ struct statx {
>> __u32 stx_dev_major; /* ID of device containing file [uncond] */
>> __u32 stx_dev_minor;
>> /* 0x90 */
>> - __u64 __spare2[14]; /* Spare space for future expansion */
>> + __u64 stx_mnt_id;
>> + __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
>> + __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
>> + /* 0xa0 */
>> + __u64 stx_subvol; /* Subvolume identifier */
>> + __u32 stx_atomic_write_unit_min; /* Min atomic write unit in bytes */
>> + __u32 stx_atomic_write_unit_max; /* Max atomic write unit in bytes */
>> + /* 0xb0 */
>> + __u32   stx_atomic_write_segments_max; /* Max atomic write segment count */
>> + __u32   __spare1[1];
>> + /* 0xb8 */
>> + __u64 __spare3[9]; /* Spare space for future expansion */
>> /* 0x100 */
>> };
>> +#endif
>> 
>> +#ifndef STATX_TYPE
>> /*
>>  * Flags to be stx_mask
>>  *
>> @@ -174,4 +190,9 @@ struct statx {
>> #define STATX_ATTR_AUTOMOUNT 0x00001000 /* Dir: Automount trigger */
>> 
>> #endif /* STATX_TYPE */
>> +
>> +#ifndef STATX_WRITE_ATOMIC
>> +#define STATX_WRITE_ATOMIC 0x00010000U /* Want/got atomic_write_* fields */
>> +#endif
>> +
>> #endif /* XFS_IO_STATX_H */
>> diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
>> index 6de8b33e..bc8a49a9 100644
>> --- a/m4/package_libcdev.m4
>> +++ b/m4/package_libcdev.m4
>> @@ -98,6 +98,26 @@ AC_DEFUN([AC_NEED_INTERNAL_FSCRYPT_POLICY_V2],
>>     AC_SUBST(need_internal_fscrypt_policy_v2)
>>   ])
>> 
>> +#
>> +# Check if we need to override the system struct statx with
>> +# the internal definition.  This /only/ happens if the system
>> +# actually defines struct statx /and/ the system definition
>> +# is missing certain fields.
>> +#
>> +AC_DEFUN([AC_NEED_INTERNAL_STATX],
>> +  [ AC_CHECK_TYPE(struct statx,
>> +      [
>> +        AC_CHECK_MEMBER(struct statx.stx_atomic_write_unit_min,
>> +          ,
>> +          need_internal_statx=yes,
>> +          [#include <linux/stat.h>]
>> +        )
>> +      ],,
>> +      [#include <linux/stat.h>]
>> +    )
>> +    AC_SUBST(need_internal_statx)
>> +  ])
>> +
>> #
>> # Check if we have a FS_IOC_GETFSMAP ioctl (Linux)
>> #
>> -- 
>> 2.34.1
>> 
>>
John Garry Nov. 19, 2024, 2:10 p.m. UTC | #3
On 18/11/2024 23:52, Catherine Hoang wrote:
>   #include "init.h"
> @@ -347,6 +351,9 @@ dump_raw_statx(struct statx *stx)
>   	printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);
>   	printf("stat.dev_major = %u\n", stx->stx_dev_major);
>   	printf("stat.dev_minor = %u\n", stx->stx_dev_minor);
> +	printf("stat.atomic_write_unit_min = %lld\n", (long long)stx->stx_atomic_write_unit_min);
> +	printf("stat.atomic_write_unit_max = %lld\n", (long long)stx->stx_atomic_write_unit_max);
> +	printf("stat.atomic_write_segments_max = %lld\n", (long long)stx->stx_atomic_write_segments_max);

Is there a special reason to do this casting to long long? We only do 
that for u64 values, I think.

Thanks,
John
Darrick J. Wong Nov. 19, 2024, 4:05 p.m. UTC | #4
On Tue, Nov 19, 2024 at 02:30:52AM +0000, Catherine Hoang wrote:
> > On Nov 18, 2024, at 4:01 PM, Darrick J. Wong <djwong@kernel.org> wrote:
> > 
> > On Mon, Nov 18, 2024 at 03:52:55PM -0800, Catherine Hoang wrote:
> >> Add support for the new atomic_write_unit_min, atomic_write_unit_max, and
> >> atomic_write_segments_max fields in statx for xfs_io. In order to support builds
> >> against old kernel headers, define our own internal statx structs. If the
> >> system's struct statx does not have the required atomic write fields, override
> >> the struct definitions with the internal definitions in statx.h.
> >> 
> >> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> >> ---
> >> configure.ac          |  1 +
> >> include/builddefs.in  |  4 ++++
> >> io/stat.c             |  7 +++++++
> >> io/statx.h            | 23 ++++++++++++++++++++++-
> >> m4/package_libcdev.m4 | 20 ++++++++++++++++++++
> >> 5 files changed, 54 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/configure.ac b/configure.ac
> >> index 33b01399..0b1ef3c3 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -146,6 +146,7 @@ AC_HAVE_COPY_FILE_RANGE
> >> AC_NEED_INTERNAL_FSXATTR
> >> AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG
> >> AC_NEED_INTERNAL_FSCRYPT_POLICY_V2
> >> +AC_NEED_INTERNAL_STATX
> >> AC_HAVE_GETFSMAP
> >> AC_HAVE_MAP_SYNC
> >> AC_HAVE_DEVMAPPER
> >> diff --git a/include/builddefs.in b/include/builddefs.in
> >> index 1647d2cd..cbc9ab0c 100644
> >> --- a/include/builddefs.in
> >> +++ b/include/builddefs.in
> >> @@ -96,6 +96,7 @@ HAVE_COPY_FILE_RANGE = @have_copy_file_range@
> >> NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
> >> NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@
> >> NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@
> >> +NEED_INTERNAL_STATX = @need_internal_statx@
> >> HAVE_GETFSMAP = @have_getfsmap@
> >> HAVE_MAP_SYNC = @have_map_sync@
> >> HAVE_DEVMAPPER = @have_devmapper@
> >> @@ -130,6 +131,9 @@ endif
> >> ifeq ($(NEED_INTERNAL_FSCRYPT_POLICY_V2),yes)
> >> PCFLAGS+= -DOVERRIDE_SYSTEM_FSCRYPT_POLICY_V2
> >> endif
> >> +ifeq ($(NEED_INTERNAL_STATX),yes)
> >> +PCFLAGS+= -DOVERRIDE_SYSTEM_STATX
> >> +endif
> >> ifeq ($(HAVE_GETFSMAP),yes)
> >> PCFLAGS+= -DHAVE_GETFSMAP
> >> endif
> >> diff --git a/io/stat.c b/io/stat.c
> >> index 0f5618f6..7d1c1c93 100644
> >> --- a/io/stat.c
> >> +++ b/io/stat.c
> >> @@ -6,6 +6,10 @@
> >>  * Portions of statx support written by David Howells (dhowells@redhat.com)
> >>  */
> >> 
> >> +#ifdef OVERRIDE_SYSTEM_STATX
> >> +#define statx sys_statx
> >> +#endif
> >> +
> >> #include "command.h"
> >> #include "input.h"
> >> #include "init.h"
> >> @@ -347,6 +351,9 @@ dump_raw_statx(struct statx *stx)
> >> printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);
> >> printf("stat.dev_major = %u\n", stx->stx_dev_major);
> >> printf("stat.dev_minor = %u\n", stx->stx_dev_minor);
> >> + printf("stat.atomic_write_unit_min = %lld\n", (long long)stx->stx_atomic_write_unit_min);
> >> + printf("stat.atomic_write_unit_max = %lld\n", (long long)stx->stx_atomic_write_unit_max);
> >> + printf("stat.atomic_write_segments_max = %lld\n", (long long)stx->stx_atomic_write_segments_max);
> >> return 0;
> >> }
> >> 
> >> diff --git a/io/statx.h b/io/statx.h
> >> index c6625ac4..d151f732 100644
> >> --- a/io/statx.h
> >> +++ b/io/statx.h
> >> @@ -61,6 +61,7 @@ struct statx_timestamp {
> >> __s32 tv_nsec;
> >> __s32 __reserved;
> >> };
> >> +#endif
> >> 
> >> /*
> >>  * Structures for the extended file attribute retrieval system call
> >> @@ -99,6 +100,8 @@ struct statx_timestamp {
> >>  * will have values installed for compatibility purposes so that stat() and
> >>  * co. can be emulated in userspace.
> >>  */
> >> +#if !defined STATX_TYPE || defined OVERRIDE_SYSTEM_STATX
> > 
> > Is there any place where STATX_TYPE is not defined but
> > OVERRIDE_SYSTEM_STATX will *also* not be defined?
> 
> I don’t think so. So I guess this could just be
> 
> #ifdef OVERRIDE_SYSTEM_STATX
> 
> Does that seem right?

It seems ok to me.

--D


> > 
> > I think the m4 macro you added sets need_internal_statx=yes for old
> > systems where there's no STATX_TYPE, because there won't be a struct
> > statx, let alone atomic_write_* fields in it, right?
> > 
> > --D
> > 
> >> +#undef statx
> >> struct statx {
> >> /* 0x00 */
> >> __u32 stx_mask; /* What results were written [uncond] */
> >> @@ -126,10 +129,23 @@ struct statx {
> >> __u32 stx_dev_major; /* ID of device containing file [uncond] */
> >> __u32 stx_dev_minor;
> >> /* 0x90 */
> >> - __u64 __spare2[14]; /* Spare space for future expansion */
> >> + __u64 stx_mnt_id;
> >> + __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
> >> + __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
> >> + /* 0xa0 */
> >> + __u64 stx_subvol; /* Subvolume identifier */
> >> + __u32 stx_atomic_write_unit_min; /* Min atomic write unit in bytes */
> >> + __u32 stx_atomic_write_unit_max; /* Max atomic write unit in bytes */
> >> + /* 0xb0 */
> >> + __u32   stx_atomic_write_segments_max; /* Max atomic write segment count */
> >> + __u32   __spare1[1];
> >> + /* 0xb8 */
> >> + __u64 __spare3[9]; /* Spare space for future expansion */
> >> /* 0x100 */
> >> };
> >> +#endif
> >> 
> >> +#ifndef STATX_TYPE
> >> /*
> >>  * Flags to be stx_mask
> >>  *
> >> @@ -174,4 +190,9 @@ struct statx {
> >> #define STATX_ATTR_AUTOMOUNT 0x00001000 /* Dir: Automount trigger */
> >> 
> >> #endif /* STATX_TYPE */
> >> +
> >> +#ifndef STATX_WRITE_ATOMIC
> >> +#define STATX_WRITE_ATOMIC 0x00010000U /* Want/got atomic_write_* fields */
> >> +#endif
> >> +
> >> #endif /* XFS_IO_STATX_H */
> >> diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
> >> index 6de8b33e..bc8a49a9 100644
> >> --- a/m4/package_libcdev.m4
> >> +++ b/m4/package_libcdev.m4
> >> @@ -98,6 +98,26 @@ AC_DEFUN([AC_NEED_INTERNAL_FSCRYPT_POLICY_V2],
> >>     AC_SUBST(need_internal_fscrypt_policy_v2)
> >>   ])
> >> 
> >> +#
> >> +# Check if we need to override the system struct statx with
> >> +# the internal definition.  This /only/ happens if the system
> >> +# actually defines struct statx /and/ the system definition
> >> +# is missing certain fields.
> >> +#
> >> +AC_DEFUN([AC_NEED_INTERNAL_STATX],
> >> +  [ AC_CHECK_TYPE(struct statx,
> >> +      [
> >> +        AC_CHECK_MEMBER(struct statx.stx_atomic_write_unit_min,
> >> +          ,
> >> +          need_internal_statx=yes,
> >> +          [#include <linux/stat.h>]
> >> +        )
> >> +      ],,
> >> +      [#include <linux/stat.h>]
> >> +    )
> >> +    AC_SUBST(need_internal_statx)
> >> +  ])
> >> +
> >> #
> >> # Check if we have a FS_IOC_GETFSMAP ioctl (Linux)
> >> #
> >> -- 
> >> 2.34.1
> >> 
> >> 
>
Darrick J. Wong Nov. 19, 2024, 4:08 p.m. UTC | #5
On Tue, Nov 19, 2024 at 02:10:59PM +0000, John Garry wrote:
> On 18/11/2024 23:52, Catherine Hoang wrote:
> >   #include "init.h"
> > @@ -347,6 +351,9 @@ dump_raw_statx(struct statx *stx)
> >   	printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);
> >   	printf("stat.dev_major = %u\n", stx->stx_dev_major);
> >   	printf("stat.dev_minor = %u\n", stx->stx_dev_minor);
> > +	printf("stat.atomic_write_unit_min = %lld\n", (long long)stx->stx_atomic_write_unit_min);
> > +	printf("stat.atomic_write_unit_max = %lld\n", (long long)stx->stx_atomic_write_unit_max);
> > +	printf("stat.atomic_write_segments_max = %lld\n", (long long)stx->stx_atomic_write_segments_max);
> 
> Is there a special reason to do this casting to long long? We only do that
> for u64 values, I think.

Yeah, I don't think it's necessary here because the stx_atomic values
are __u32.  xfsprogs has a tendency to cast to explicit C types to avoid
lint warnings, particularly because 64-bit values are grossly typedef'd
depending on how long long is.

Now that I see it -- those should be %u and casts to unsigned int.

--D

> Thanks,
> John
>
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index 33b01399..0b1ef3c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -146,6 +146,7 @@  AC_HAVE_COPY_FILE_RANGE
 AC_NEED_INTERNAL_FSXATTR
 AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG
 AC_NEED_INTERNAL_FSCRYPT_POLICY_V2
+AC_NEED_INTERNAL_STATX
 AC_HAVE_GETFSMAP
 AC_HAVE_MAP_SYNC
 AC_HAVE_DEVMAPPER
diff --git a/include/builddefs.in b/include/builddefs.in
index 1647d2cd..cbc9ab0c 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -96,6 +96,7 @@  HAVE_COPY_FILE_RANGE = @have_copy_file_range@
 NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
 NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@
 NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@
+NEED_INTERNAL_STATX = @need_internal_statx@
 HAVE_GETFSMAP = @have_getfsmap@
 HAVE_MAP_SYNC = @have_map_sync@
 HAVE_DEVMAPPER = @have_devmapper@
@@ -130,6 +131,9 @@  endif
 ifeq ($(NEED_INTERNAL_FSCRYPT_POLICY_V2),yes)
 PCFLAGS+= -DOVERRIDE_SYSTEM_FSCRYPT_POLICY_V2
 endif
+ifeq ($(NEED_INTERNAL_STATX),yes)
+PCFLAGS+= -DOVERRIDE_SYSTEM_STATX
+endif
 ifeq ($(HAVE_GETFSMAP),yes)
 PCFLAGS+= -DHAVE_GETFSMAP
 endif
diff --git a/io/stat.c b/io/stat.c
index 0f5618f6..7d1c1c93 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -6,6 +6,10 @@ 
  * Portions of statx support written by David Howells (dhowells@redhat.com)
  */
 
+#ifdef OVERRIDE_SYSTEM_STATX
+#define statx sys_statx
+#endif
+
 #include "command.h"
 #include "input.h"
 #include "init.h"
@@ -347,6 +351,9 @@  dump_raw_statx(struct statx *stx)
 	printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);
 	printf("stat.dev_major = %u\n", stx->stx_dev_major);
 	printf("stat.dev_minor = %u\n", stx->stx_dev_minor);
+	printf("stat.atomic_write_unit_min = %lld\n", (long long)stx->stx_atomic_write_unit_min);
+	printf("stat.atomic_write_unit_max = %lld\n", (long long)stx->stx_atomic_write_unit_max);
+	printf("stat.atomic_write_segments_max = %lld\n", (long long)stx->stx_atomic_write_segments_max);
 	return 0;
 }
 
diff --git a/io/statx.h b/io/statx.h
index c6625ac4..d151f732 100644
--- a/io/statx.h
+++ b/io/statx.h
@@ -61,6 +61,7 @@  struct statx_timestamp {
 	__s32	tv_nsec;
 	__s32	__reserved;
 };
+#endif
 
 /*
  * Structures for the extended file attribute retrieval system call
@@ -99,6 +100,8 @@  struct statx_timestamp {
  * will have values installed for compatibility purposes so that stat() and
  * co. can be emulated in userspace.
  */
+#if !defined STATX_TYPE || defined OVERRIDE_SYSTEM_STATX
+#undef statx
 struct statx {
 	/* 0x00 */
 	__u32	stx_mask;	/* What results were written [uncond] */
@@ -126,10 +129,23 @@  struct statx {
 	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
 	__u32	stx_dev_minor;
 	/* 0x90 */
-	__u64	__spare2[14];	/* Spare space for future expansion */
+	__u64	stx_mnt_id;
+	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
+	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
+	/* 0xa0 */
+	__u64	stx_subvol;	/* Subvolume identifier */
+	__u32	stx_atomic_write_unit_min;	/* Min atomic write unit in bytes */
+	__u32	stx_atomic_write_unit_max;	/* Max atomic write unit in bytes */
+	/* 0xb0 */
+	__u32   stx_atomic_write_segments_max;	/* Max atomic write segment count */
+	__u32   __spare1[1];
+	/* 0xb8 */
+	__u64	__spare3[9];	/* Spare space for future expansion */
 	/* 0x100 */
 };
+#endif
 
+#ifndef STATX_TYPE
 /*
  * Flags to be stx_mask
  *
@@ -174,4 +190,9 @@  struct statx {
 #define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
 
 #endif /* STATX_TYPE */
+
+#ifndef STATX_WRITE_ATOMIC
+#define STATX_WRITE_ATOMIC	0x00010000U	/* Want/got atomic_write_* fields */
+#endif
+
 #endif /* XFS_IO_STATX_H */
diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
index 6de8b33e..bc8a49a9 100644
--- a/m4/package_libcdev.m4
+++ b/m4/package_libcdev.m4
@@ -98,6 +98,26 @@  AC_DEFUN([AC_NEED_INTERNAL_FSCRYPT_POLICY_V2],
     AC_SUBST(need_internal_fscrypt_policy_v2)
   ])
 
+#
+# Check if we need to override the system struct statx with
+# the internal definition.  This /only/ happens if the system
+# actually defines struct statx /and/ the system definition
+# is missing certain fields.
+#
+AC_DEFUN([AC_NEED_INTERNAL_STATX],
+  [ AC_CHECK_TYPE(struct statx,
+      [
+        AC_CHECK_MEMBER(struct statx.stx_atomic_write_unit_min,
+          ,
+          need_internal_statx=yes,
+          [#include <linux/stat.h>]
+        )
+      ],,
+      [#include <linux/stat.h>]
+    )
+    AC_SUBST(need_internal_statx)
+  ])
+
 #
 # Check if we have a FS_IOC_GETFSMAP ioctl (Linux)
 #