On Tue, Nov 30 2021 at 00:37:03 -0600, Clay Harris quoth thus: > On Mon, Nov 29 2021 at 20:16:02 -0700, Andreas Dilger quoth thus: > > > > > > On Nov 29, 2021, at 6:08 PM, Clay Harris <bugs@xxxxxxxxxxx> wrote: > > > > > > On Mon, Nov 29 2021 at 14:12:52 -0800, Stefan Roesch quoth thus: > > > > > >> This adds the xattr support to io_uring. The intent is to have a more > > >> complete support for file operations in io_uring. > > >> > > >> This change adds support for the following functions to io_uring: > > >> - fgetxattr > > >> - fsetxattr > > >> - getxattr > > >> - setxattr > > > > > > You may wish to consider the following. > > > > > > Patching for these functions makes for an excellent opportunity > > > to provide a better interface. Rather than implement fXetattr > > > at all, you could enable io_uring to use functions like: > > > > > > int Xetxattr(int dfd, const char *path, const char *name, > > > [const] void *value, size_t size, int flags); > > > > This would naturally be named "...xattrat()"? > > Indeed! > > > > Not only does this simplify the io_uring interface down to two > > > functions, but modernizes and fixes a deficit in usability. > > > In terms of io_uring, this is just changing internal interfaces. > > > > Even better would be the ability to get/set an array of xattrs in > > one call, to avoid repeated path lookups in the common case of > > handling multiple xattrs on a single file. > > True. > > > > Although unnecessary for io_uring, it would be nice to at least > > > consider what parts of this code could be leveraged for future > > > Xetxattr2 syscalls. > s/Xetxattr2/Xetxattrat/ I forgot to mention a final thought about the interface. Unless there is a really good reason (security auditing??), there is no reason to have a removexattr() function. That seems much better handled by passing NULL for value and specifying a remove flag in flags to setxattrat(). > > > > > >> Patch 1: fs: make user_path_at_empty() take a struct filename > > >> The user_path_at_empty filename parameter has been changed > > >> from a const char user pointer to a filename struct. io_uring > > >> operates on filenames. > > >> In addition also the functions that call user_path_at_empty > > >> in namei.c and stat.c have been modified for this change. > > >> > > >> Patch 2: fs: split off setxattr_setup function from setxattr > > >> Split off the setup part of the setxattr function > > >> > > >> Patch 3: fs: split off the vfs_getxattr from getxattr > > >> Split of the vfs_getxattr part from getxattr. This will > > >> allow to invoke it from io_uring. > > >> > > >> Patch 4: io_uring: add fsetxattr and setxattr support > > >> This adds new functions to support the fsetxattr and setxattr > > >> functions. > > >> > > >> Patch 5: io_uring: add fgetxattr and getxattr support > > >> This adds new functions to support the fgetxattr and getxattr > > >> functions. > > >> > > >> > > >> There are two additional patches: > > >> liburing: Add support for xattr api's. > > >> This also includes the tests for the new code. > > >> xfstests: Add support for io_uring xattr support. > > >> > > >> > > >> Stefan Roesch (5): > > >> fs: make user_path_at_empty() take a struct filename > > >> fs: split off setxattr_setup function from setxattr > > >> fs: split off the vfs_getxattr from getxattr > > >> io_uring: add fsetxattr and setxattr support > > >> io_uring: add fgetxattr and getxattr support > > >> > > >> fs/internal.h | 23 +++ > > >> fs/io_uring.c | 325 ++++++++++++++++++++++++++++++++++ > > >> fs/namei.c | 5 +- > > >> fs/stat.c | 7 +- > > >> fs/xattr.c | 114 +++++++----- > > >> include/linux/namei.h | 4 +- > > >> include/uapi/linux/io_uring.h | 8 +- > > >> 7 files changed, 439 insertions(+), 47 deletions(-) > > >> > > >> > > >> Signed-off-by: Stefan Roesch <shr@xxxxxx> > > >> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb > > >> -- > > >> 2.30.2 > > > > > > Cheers, Andreas > > > > > > > > > > >
