| Message ID | 20230810105501.1418427-3-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse: support birth time | expand |
On 8/10/23 12:54, Miklos Szeredi wrote: > Use the same structure as statx. Wouldn't it be easier to just include struct statx? Or is there an issue with __u32, etc? If so, just a sufficiently large array could be used and statx values just mem-copied in/out? > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > include/uapi/linux/fuse.h | 56 ++++++++++++++++++++++++++++++++++++++- > 1 file changed, 55 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h > index b3fcab13fcd3..fe700b91b33b 100644 > --- a/include/uapi/linux/fuse.h > +++ b/include/uapi/linux/fuse.h > @@ -207,6 +207,9 @@ > * - add FUSE_EXT_GROUPS > * - add FUSE_CREATE_SUPP_GROUP > * - add FUSE_HAS_EXPIRE_ONLY > + * > + * 7.39 > + * - add FUSE_STATX and related structures > */ > > #ifndef _LINUX_FUSE_H > @@ -242,7 +245,7 @@ > #define FUSE_KERNEL_VERSION 7 > > /** Minor version number of this interface */ > -#define FUSE_KERNEL_MINOR_VERSION 38 > +#define FUSE_KERNEL_MINOR_VERSION 39 > > /** The node ID of the root inode */ > #define FUSE_ROOT_ID 1 > @@ -269,6 +272,40 @@ struct fuse_attr { > uint32_t flags; > }; > > +/* > + * The following structures are bit-for-bit compatible with the statx(2) ABI in > + * Linux. > + */ > +struct fuse_sx_time { > + int64_t tv_sec; > + uint32_t tv_nsec; > + int32_t __reserved; > +}; > + > +struct fuse_statx { > + uint32_t mask; > + uint32_t blksize; > + uint64_t attributes; > + uint32_t nlink; > + uint32_t uid; > + uint32_t gid; > + uint16_t mode; > + uint16_t __spare0[1]; > + uint64_t ino; > + uint64_t size; > + uint64_t blocks; > + uint64_t attributes_mask; > + struct fuse_sx_time atime; > + struct fuse_sx_time btime; > + struct fuse_sx_time ctime; > + struct fuse_sx_time mtime; > + uint32_t rdev_major; > + uint32_t rdev_minor; > + uint32_t dev_major; > + uint32_t dev_minor; > + uint64_t __spare2[14]; > +}; Looks like some recent values are missing? /* 0x90 */ __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 __spare3[12]; /* Spare space for future expansion */ /* 0x100 */ Which is basically why my personal preference would be not to do have a copy of the struct - there is maintenance overhead. > + > struct fuse_kstatfs { > uint64_t blocks; > uint64_t bfree; > @@ -575,6 +612,7 @@ enum fuse_opcode { > FUSE_REMOVEMAPPING = 49, > FUSE_SYNCFS = 50, > FUSE_TMPFILE = 51, > + FUSE_STATX = 52, > > /* CUSE specific operations */ > CUSE_INIT = 4096, > @@ -639,6 +677,22 @@ struct fuse_attr_out { > struct fuse_attr attr; > }; > > +struct fuse_statx_in { > + uint32_t getattr_flags; > + uint32_t reserved; > + uint64_t fh; > + uint32_t sx_flags; > + uint32_t sx_mask; > +}; > + > +struct fuse_statx_out { > + uint64_t attr_valid; /* Cache timeout for the attributes */ > + uint32_t attr_valid_nsec; > + uint32_t flags; > + uint64_t spare[2]; > + struct fuse_statx stat; > +}; > + > #define FUSE_COMPAT_MKNOD_IN_SIZE 8 > > struct fuse_mknod_in { Thanks, Bernd
On Thu, 10 Aug 2023 at 15:23, Bernd Schubert <bernd.schubert@fastmail.fm> wrote: > > > > On 8/10/23 12:54, Miklos Szeredi wrote: > > Use the same structure as statx. > > Wouldn't it be easier to just include struct statx? Or is there an issue > with __u32, etc? If so, just a sufficiently large array could be used > and statx values just mem-copied in/out? <linux/uapi/fuse.h> is OS independent. Ports can grab it and use it in their userspace and kernel implementations. > > > > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > > --- > > include/uapi/linux/fuse.h | 56 ++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 55 insertions(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h > > index b3fcab13fcd3..fe700b91b33b 100644 > > --- a/include/uapi/linux/fuse.h > > +++ b/include/uapi/linux/fuse.h > > @@ -207,6 +207,9 @@ > > * - add FUSE_EXT_GROUPS > > * - add FUSE_CREATE_SUPP_GROUP > > * - add FUSE_HAS_EXPIRE_ONLY > > + * > > + * 7.39 > > + * - add FUSE_STATX and related structures > > */ > > > > #ifndef _LINUX_FUSE_H > > @@ -242,7 +245,7 @@ > > #define FUSE_KERNEL_VERSION 7 > > > > /** Minor version number of this interface */ > > -#define FUSE_KERNEL_MINOR_VERSION 38 > > +#define FUSE_KERNEL_MINOR_VERSION 39 > > > > /** The node ID of the root inode */ > > #define FUSE_ROOT_ID 1 > > @@ -269,6 +272,40 @@ struct fuse_attr { > > uint32_t flags; > > }; > > > > +/* > > + * The following structures are bit-for-bit compatible with the statx(2) ABI in > > + * Linux. > > + */ > > +struct fuse_sx_time { > > + int64_t tv_sec; > > + uint32_t tv_nsec; > > + int32_t __reserved; > > +}; > > + > > +struct fuse_statx { > > + uint32_t mask; > > + uint32_t blksize; > > + uint64_t attributes; > > + uint32_t nlink; > > + uint32_t uid; > > + uint32_t gid; > > + uint16_t mode; > > + uint16_t __spare0[1]; > > + uint64_t ino; > > + uint64_t size; > > + uint64_t blocks; > > + uint64_t attributes_mask; > > + struct fuse_sx_time atime; > > + struct fuse_sx_time btime; > > + struct fuse_sx_time ctime; > > + struct fuse_sx_time mtime; > > + uint32_t rdev_major; > > + uint32_t rdev_minor; > > + uint32_t dev_major; > > + uint32_t dev_minor; > > + uint64_t __spare2[14]; > > +}; > > Looks like some recent values are missing? It doesn't matter, since those parts are not used. > > /* 0x90 */ > __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 __spare3[12]; /* Spare space for future expansion */ > /* 0x100 */ > > Which is basically why my personal preference would be not to do have a > copy of the struct - there is maintenance overhead. Whenever the new fields would be used in the kernel the fields can be added. So no need to continually update the one in fuse, since those fields cannot be referenced by the kernel. Userspace might actually access those fields through struct statx, but that's okay, the interface was designed so that the producer and consumer can use different versions of the API. Thanks, Miklos
On 8/10/23 16:08, Miklos Szeredi wrote: > On Thu, 10 Aug 2023 at 15:23, Bernd Schubert <bernd.schubert@fastmail.fm> wrote: >> >> >> >> On 8/10/23 12:54, Miklos Szeredi wrote: >>> Use the same structure as statx. >> >> Wouldn't it be easier to just include struct statx? Or is there an issue >> with __u32, etc? If so, just a sufficiently large array could be used >> and statx values just mem-copied in/out? > > <linux/uapi/fuse.h> is OS independent. Ports can grab it and use it > in their userspace and kernel implementations. Ok, but why not just like this? struct fuse_statx { uint8_t fuse_statx_values[256]; } struct fuse_statx fuse_statx; struct statx *statx_ptr = (struct statx *)&fuse_statx.fuse_statx_values; Hmm, I see an issue if that struct is passed over network and different OS are involved, which _might_ have different structs or sizes. So yeah, the copy approach is safest. Although as fuse_statx is between kernel and userspace only - it should not matter? > > >> >>> >>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> >>> --- >>> include/uapi/linux/fuse.h | 56 ++++++++++++++++++++++++++++++++++++++- >>> 1 file changed, 55 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h >>> index b3fcab13fcd3..fe700b91b33b 100644 >>> --- a/include/uapi/linux/fuse.h >>> +++ b/include/uapi/linux/fuse.h >>> @@ -207,6 +207,9 @@ >>> * - add FUSE_EXT_GROUPS >>> * - add FUSE_CREATE_SUPP_GROUP >>> * - add FUSE_HAS_EXPIRE_ONLY >>> + * >>> + * 7.39 >>> + * - add FUSE_STATX and related structures >>> */ >>> >>> #ifndef _LINUX_FUSE_H >>> @@ -242,7 +245,7 @@ >>> #define FUSE_KERNEL_VERSION 7 >>> >>> /** Minor version number of this interface */ >>> -#define FUSE_KERNEL_MINOR_VERSION 38 >>> +#define FUSE_KERNEL_MINOR_VERSION 39 >>> >>> /** The node ID of the root inode */ >>> #define FUSE_ROOT_ID 1 >>> @@ -269,6 +272,40 @@ struct fuse_attr { >>> uint32_t flags; >>> }; >>> >>> +/* >>> + * The following structures are bit-for-bit compatible with the statx(2) ABI in >>> + * Linux. >>> + */ >>> +struct fuse_sx_time { >>> + int64_t tv_sec; >>> + uint32_t tv_nsec; >>> + int32_t __reserved; >>> +}; >>> + >>> +struct fuse_statx { >>> + uint32_t mask; >>> + uint32_t blksize; >>> + uint64_t attributes; >>> + uint32_t nlink; >>> + uint32_t uid; >>> + uint32_t gid; >>> + uint16_t mode; >>> + uint16_t __spare0[1]; >>> + uint64_t ino; >>> + uint64_t size; >>> + uint64_t blocks; >>> + uint64_t attributes_mask; >>> + struct fuse_sx_time atime; >>> + struct fuse_sx_time btime; >>> + struct fuse_sx_time ctime; >>> + struct fuse_sx_time mtime; >>> + uint32_t rdev_major; >>> + uint32_t rdev_minor; >>> + uint32_t dev_major; >>> + uint32_t dev_minor; >>> + uint64_t __spare2[14]; >>> +}; >> >> Looks like some recent values are missing? > > It doesn't matter, since those parts are not used. > >> >> /* 0x90 */ >> __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 __spare3[12]; /* Spare space for future expansion */ >> /* 0x100 */ >> >> Which is basically why my personal preference would be not to do have a >> copy of the struct - there is maintenance overhead. > > Whenever the new fields would be used in the kernel the fields can be > added. So no need to continually update the one in fuse, since those Maybe I'm over optimizing, I just see that userspace side then also needs an updated struct - which can be easily forgotten. While plain struct statx would't have that issue. Thanks, Bernd
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h index b3fcab13fcd3..fe700b91b33b 100644 --- a/include/uapi/linux/fuse.h +++ b/include/uapi/linux/fuse.h @@ -207,6 +207,9 @@ * - add FUSE_EXT_GROUPS * - add FUSE_CREATE_SUPP_GROUP * - add FUSE_HAS_EXPIRE_ONLY + * + * 7.39 + * - add FUSE_STATX and related structures */ #ifndef _LINUX_FUSE_H @@ -242,7 +245,7 @@ #define FUSE_KERNEL_VERSION 7 /** Minor version number of this interface */ -#define FUSE_KERNEL_MINOR_VERSION 38 +#define FUSE_KERNEL_MINOR_VERSION 39 /** The node ID of the root inode */ #define FUSE_ROOT_ID 1 @@ -269,6 +272,40 @@ struct fuse_attr { uint32_t flags; }; +/* + * The following structures are bit-for-bit compatible with the statx(2) ABI in + * Linux. + */ +struct fuse_sx_time { + int64_t tv_sec; + uint32_t tv_nsec; + int32_t __reserved; +}; + +struct fuse_statx { + uint32_t mask; + uint32_t blksize; + uint64_t attributes; + uint32_t nlink; + uint32_t uid; + uint32_t gid; + uint16_t mode; + uint16_t __spare0[1]; + uint64_t ino; + uint64_t size; + uint64_t blocks; + uint64_t attributes_mask; + struct fuse_sx_time atime; + struct fuse_sx_time btime; + struct fuse_sx_time ctime; + struct fuse_sx_time mtime; + uint32_t rdev_major; + uint32_t rdev_minor; + uint32_t dev_major; + uint32_t dev_minor; + uint64_t __spare2[14]; +}; + struct fuse_kstatfs { uint64_t blocks; uint64_t bfree; @@ -575,6 +612,7 @@ enum fuse_opcode { FUSE_REMOVEMAPPING = 49, FUSE_SYNCFS = 50, FUSE_TMPFILE = 51, + FUSE_STATX = 52, /* CUSE specific operations */ CUSE_INIT = 4096, @@ -639,6 +677,22 @@ struct fuse_attr_out { struct fuse_attr attr; }; +struct fuse_statx_in { + uint32_t getattr_flags; + uint32_t reserved; + uint64_t fh; + uint32_t sx_flags; + uint32_t sx_mask; +}; + +struct fuse_statx_out { + uint64_t attr_valid; /* Cache timeout for the attributes */ + uint32_t attr_valid_nsec; + uint32_t flags; + uint64_t spare[2]; + struct fuse_statx stat; +}; + #define FUSE_COMPAT_MKNOD_IN_SIZE 8 struct fuse_mknod_in {
Use the same structure as statx. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- include/uapi/linux/fuse.h | 56 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-)