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

Er... "fsx_fileattr" is the struct that the system call uses?

That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.

I dunno.  There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.

Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:

struct mount_attr vs struct mount_kattr

struct clone_args vs struct kernel_clone_kargs

etc.

More whining/bikeshedding to come.

<snip stuff that looks ok to me>

<<well, I still dislike the CLASS(fd, fd)(fd) syntax...>>

Noted, and duly ignored...

quoted

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

Agreed. I changed that.