On 11/29/21 7:16 PM, Andreas Dilger wrote: > >> 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()"? > >> 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. > You are proposing a new API. However that API has its challenges: - How do you implement error handling? What if only some requests fail. - It will make the code considerably more complicated (for user-space as well as kernel) Instead the user can do the following: - io_uring already has support for the following: - io_uring already has the ability to prepare several SQE's at once - These SQE's can be submitted in one operation - The SQE's can also be linked and waited for as a unit. - Allows to map each individual CQE to its request. >> 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. > >> >>> 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 > > > > >
