Re: [PATCH v6 6/6] fs: introduce file_getattr and file_setattr syscalls

[Darrick J. Wong]

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. :)

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?

--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
> > > 
> > > 
>