On Tuesday 01 July 2025 12:08:57 Darrick J. Wong wrote: > On Tue, Jul 01, 2025 at 08:54:57PM +0200, Pali Rohár wrote: > > On Tuesday 01 July 2025 11:43:17 Darrick J. Wong wrote: > > > On Mon, Jun 30, 2025 at 06:20:16PM +0200, Andrey Albershteyn wrote: > > > > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h > > > > index 0098b0ce8ccb..0784f2033ba4 100644 > > > > --- a/include/uapi/linux/fs.h > > > > +++ b/include/uapi/linux/fs.h > > > > @@ -148,6 +148,24 @@ struct fsxattr { > > > > unsigned char fsx_pad[8]; > > > > }; > > > > > > > > +/* > > > > + * Variable size structure for file_[sg]et_attr(). > > > > + * > > > > + * Note. This is alternative to the structure 'struct fileattr'/'struct fsxattr'. > > > > + * As this structure is passed to/from userspace with its size, this can > > > > + * be versioned based on the size. > > > > + */ > > > > +struct fsx_fileattr { > > > > + __u32 fsx_xflags; /* xflags field value (get/set) */ > > > > > > Should this to be __u64 from the start? Seeing as (a) this struct is > > > not already a multiple of 8 bytes and (b) it's likely that we'll have to > > > add a u64 field at some point. That would also address brauner's > > > comment about padding. > > > > Hello! > > > > As I have already mentioned, after this syscall API/ABI is finished, I'm > > planning to prepare patches for changing just selected fields / flags by > > introducing a new mask field, and support for additional flags used by > > existing filesystems (like windows flags). > > > > My idea is extending this structure for a new "u32 fsx_xflags_mask" > > and new "u32 fsx_xflags2" + "u32 fsx_xflags2_mask". (field names are > > just examples). > > > > So in case you are extending the structure now, please consider if it > > makes sense to add all members, so we do not have to define 2 or 3 > > structure versions in near feature. > > > > Your idea of __u64 for fsx_xflags means that it will already cover the > > "u32 fsx_xflags2" field. > > Ah, ok, so that work *is* still coming. :) Yes. I'm just waiting until this patch series is accepted. In past I have already sent RFC patches to the list which modifies the existing ioctl interface. So you can look at it if you want :-) > Are you still planning to add masks for xflags bits that are clearable > and settable? i.e. > > __u64 fa_xflags; /* state */ > ... > <end of V0 structure> > > __u64 fa_xflags_mask; /* bits for setattr to examine */ > __u64 fa_xflags_clearable; /* clearable bits */ > __u64 fa_xflags_settable; /* settable bits */ > > I think it's easier just to define u64 in the V0 structure and then add > the three new fields in V1. What do you think? I wanted the interface which would allow to atomically change specified bit/flag without the need for get-modify-set. And I think that this would not work as the fa_xflags requires the state. My idea is following: __u64 fa_xflags; ... <end of V0 structure> __u64 fa_xflags_mask; The fa_xflags_mask will specify which bits from the fa_xflags and from other fa_* fields in V0 struct are going to be changed. > --D > > > > --D > > > > > > > + __u32 fsx_extsize; /* extsize field value (get/set)*/ > > > > + __u32 fsx_nextents; /* nextents field value (get) */ > > > > + __u32 fsx_projid; /* project identifier (get/set) */ > > > > + __u32 fsx_cowextsize; /* CoW extsize field value (get/set) */ > > > > +}; > > > > + > > > > +#define FSX_FILEATTR_SIZE_VER0 20 > > > > +#define FSX_FILEATTR_SIZE_LATEST FSX_FILEATTR_SIZE_VER0 > > > > + > > > > /* > > > > * Flags for the fsx_xflags field > > > > */ > > > > diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl > > > > index 580b4e246aec..d1ae5e92c615 100644 > > > > --- a/scripts/syscall.tbl > > > > +++ b/scripts/syscall.tbl > > > > @@ -408,3 +408,5 @@ > > > > 465 common listxattrat sys_listxattrat > > > > 466 common removexattrat sys_removexattrat > > > > 467 common open_tree_attr sys_open_tree_attr > > > > +468 common file_getattr sys_file_getattr > > > > +469 common file_setattr sys_file_setattr > > > > > > > > -- > > > > 2.47.2 > > > > > > > > > >