* [PATCH v1 1/5] fs: make user_path_at_empty() take a struct filename
2021-11-29 22:12 [PATCH v1 0/5] io_uring: add xattr support Stefan Roesch
@ 2021-11-29 22:12 ` Stefan Roesch
2021-11-30 2:09 ` kernel test robot
2021-11-29 22:12 ` [PATCH v1 2/5] fs: split off setxattr_setup function from setxattr Stefan Roesch
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Stefan Roesch @ 2021-11-29 22:12 UTC (permalink / raw)
To: io-uring, linux-fsdevel; +Cc: shr
Summary:
- Changes the user_path_at_empty function to take a filename
struct instead of an user character pointer.
- It also includes the necessary changes in stat.c and namei.c
to call the user_path_at_empty function.
Signed-off-by: Stefan Roesch <[email protected]>
---
fs/namei.c | 5 ++---
fs/stat.c | 7 ++++++-
include/linux/namei.h | 4 ++--
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 1f9d2187c765..baf34cde9ecd 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2794,10 +2794,9 @@ int path_pts(struct path *path)
}
#endif
-int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
- struct path *path, int *empty)
+int user_path_at_empty(int dfd, struct filename *filename, unsigned flags,
+ struct path *path)
{
- struct filename *filename = getname_flags(name, flags, empty);
int ret = filename_lookup(dfd, filename, flags, path, NULL);
putname(filename);
diff --git a/fs/stat.c b/fs/stat.c
index 28d2020ba1f4..d8752c103062 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -435,12 +435,17 @@ static int do_readlinkat(int dfd, const char __user *pathname,
int error;
int empty = 0;
unsigned int lookup_flags = LOOKUP_EMPTY;
+ struct filename *filename;
if (bufsiz <= 0)
return -EINVAL;
retry:
- error = user_path_at_empty(dfd, pathname, lookup_flags, &path, &empty);
+ filename = getname_flags(pathname, lookup_flags, &empty);
+ if (IS_ERR(filename))
+ return PTR_ERR(filename);
+
+ error = user_path_at_empty(dfd, filename, lookup_flags, &path);
if (!error) {
struct inode *inode = d_backing_inode(path.dentry);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index e89329bb3134..dc1ae29478b0 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -49,12 +49,12 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
extern int path_pts(struct path *path);
-extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
+extern int user_path_at_empty(int, struct filename *, unsigned, struct path *);
static inline int user_path_at(int dfd, const char __user *name, unsigned flags,
struct path *path)
{
- return user_path_at_empty(dfd, name, flags, path, NULL);
+ return user_path_at_empty(dfd, getname_flags(name, flags, NULL), flags, path);
}
extern int kern_path(const char *, unsigned, struct path *);
--
2.30.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 2/5] fs: split off setxattr_setup function from setxattr
2021-11-29 22:12 [PATCH v1 0/5] io_uring: add xattr support Stefan Roesch
2021-11-29 22:12 ` [PATCH v1 1/5] fs: make user_path_at_empty() take a struct filename Stefan Roesch
@ 2021-11-29 22:12 ` Stefan Roesch
2021-11-29 22:12 ` [PATCH v1 3/5] fs: split off the vfs_getxattr from getxattr Stefan Roesch
` (3 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Roesch @ 2021-11-29 22:12 UTC (permalink / raw)
To: io-uring, linux-fsdevel; +Cc: shr
Summary:
This splits of the setup part of the function
setxattr in its own dedicated function called
setxattr_setup.
This makes it possible to call this function
from io_uring in the pre-processing of an
xattr request.
Signed-off-by: Stefan Roesch <[email protected]>
---
fs/internal.h | 17 ++++++++++++
fs/xattr.c | 77 ++++++++++++++++++++++++++++++++++-----------------
2 files changed, 68 insertions(+), 26 deletions(-)
diff --git a/fs/internal.h b/fs/internal.h
index 7979ff8d168c..c5c82bfb5ecf 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -194,3 +194,20 @@ long splice_file_to_pipe(struct file *in,
struct pipe_inode_info *opipe,
loff_t *offset,
size_t len, unsigned int flags);
+
+ /*
+ * fs/xattr.c:
+ */
+struct xattr_ctx {
+ /* Value of attribute */
+ const void __user *value;
+ size_t size;
+ /* Attribute name */
+ char *name;
+ int name_sz;
+ int flags;
+};
+
+void *setxattr_setup(struct user_namespace *mnt_userns,
+ const char __user *name,
+ struct xattr_ctx *data);
diff --git a/fs/xattr.c b/fs/xattr.c
index 5c8c5175b385..13963b914ac5 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -25,6 +25,8 @@
#include <linux/uaccess.h>
+#include "internal.h"
+
static const char *
strcmp_prefix(const char *a, const char *a_prefix)
{
@@ -539,43 +541,66 @@ EXPORT_SYMBOL_GPL(vfs_removexattr);
/*
* Extended attribute SET operations
*/
-static long
-setxattr(struct user_namespace *mnt_userns, struct dentry *d,
- const char __user *name, const void __user *value, size_t size,
- int flags)
+
+void *setxattr_setup(struct user_namespace *mnt_userns, const char __user *name,
+ struct xattr_ctx *ctx)
{
+ void *ret = NULL;
int error;
- void *kvalue = NULL;
- char kname[XATTR_NAME_MAX + 1];
- if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
- return -EINVAL;
+ if (ctx->flags & ~(XATTR_CREATE|XATTR_REPLACE))
+ return ERR_PTR(-EINVAL);
- error = strncpy_from_user(kname, name, sizeof(kname));
- if (error == 0 || error == sizeof(kname))
- error = -ERANGE;
+ error = strncpy_from_user(ctx->name, name, ctx->name_sz);
+ if (error == 0 || error == ctx->name_sz)
+ return ERR_PTR(-ERANGE);
if (error < 0)
- return error;
+ return ERR_PTR(error);
- if (size) {
- if (size > XATTR_SIZE_MAX)
- return -E2BIG;
- kvalue = kvmalloc(size, GFP_KERNEL);
- if (!kvalue)
- return -ENOMEM;
- if (copy_from_user(kvalue, value, size)) {
- error = -EFAULT;
- goto out;
+ if (ctx->size) {
+ if (ctx->size > XATTR_SIZE_MAX)
+ return ERR_PTR(-E2BIG);
+
+ ret = kvmalloc(ctx->size, GFP_KERNEL);
+ if (!ret)
+ return ERR_PTR(-ENOMEM);
+
+ if (copy_from_user(ret, ctx->value, ctx->size)) {
+ kfree(ret);
+ return ERR_PTR(-EFAULT);
}
- if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
- (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
- posix_acl_fix_xattr_from_user(mnt_userns, kvalue, size);
+
+ if ((strcmp(ctx->name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
+ (strcmp(ctx->name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
+ posix_acl_fix_xattr_from_user(mnt_userns, ret, ctx->size);
}
+ return ret;
+}
+
+static long
+setxattr(struct user_namespace *mnt_userns, struct dentry *d,
+ const char __user *name, const void __user *value, size_t size,
+ int flags)
+{
+ char kname[XATTR_NAME_MAX + 1];
+ struct xattr_ctx ctx = {
+ .value = value,
+ .size = size,
+ .name = kname,
+ .name_sz = sizeof(kname),
+ .flags = flags,
+ };
+ void *kvalue;
+ int error;
+
+ kvalue = setxattr_setup(mnt_userns, name, &ctx);
+ if (IS_ERR(kvalue))
+ return PTR_ERR(kvalue);
+
error = vfs_setxattr(mnt_userns, d, kname, kvalue, size, flags);
-out:
- kvfree(kvalue);
+ kvfree(kvalue);
return error;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 3/5] fs: split off the vfs_getxattr from getxattr
2021-11-29 22:12 [PATCH v1 0/5] io_uring: add xattr support Stefan Roesch
2021-11-29 22:12 ` [PATCH v1 1/5] fs: make user_path_at_empty() take a struct filename Stefan Roesch
2021-11-29 22:12 ` [PATCH v1 2/5] fs: split off setxattr_setup function from setxattr Stefan Roesch
@ 2021-11-29 22:12 ` Stefan Roesch
2021-11-29 22:12 ` [PATCH v1 4/5] io_uring: add fsetxattr and setxattr support Stefan Roesch
` (2 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Roesch @ 2021-11-29 22:12 UTC (permalink / raw)
To: io-uring, linux-fsdevel; +Cc: shr
Summary:
This splits off vfs_getxattr call from the getxattr
function. This will allow io_uring to call it from
its io worker.
Signed-off-by: Stefan Roesch <[email protected]>
---
fs/internal.h | 6 ++++++
fs/xattr.c | 37 +++++++++++++++++++++++--------------
2 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/fs/internal.h b/fs/internal.h
index c5c82bfb5ecf..9805415b199c 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -208,6 +208,12 @@ struct xattr_ctx {
int flags;
};
+ssize_t do_getxattr(struct user_namespace *mnt_userns,
+ struct dentry *d,
+ const char *kname,
+ void __user *value,
+ size_t size);
+
void *setxattr_setup(struct user_namespace *mnt_userns,
const char __user *name,
struct xattr_ctx *data);
diff --git a/fs/xattr.c b/fs/xattr.c
index 13963b914ac5..9e1dba601a03 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -666,19 +666,12 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
/*
* Extended attribute GET operations
*/
-static ssize_t
-getxattr(struct user_namespace *mnt_userns, struct dentry *d,
- const char __user *name, void __user *value, size_t size)
+ssize_t
+do_getxattr(struct user_namespace *mnt_userns, struct dentry *d,
+ const char *name, void __user *value, size_t size)
{
- ssize_t error;
void *kvalue = NULL;
- char kname[XATTR_NAME_MAX + 1];
-
- error = strncpy_from_user(kname, name, sizeof(kname));
- if (error == 0 || error == sizeof(kname))
- error = -ERANGE;
- if (error < 0)
- return error;
+ size_t error;
if (size) {
if (size > XATTR_SIZE_MAX)
@@ -688,10 +681,10 @@ getxattr(struct user_namespace *mnt_userns, struct dentry *d,
return -ENOMEM;
}
- error = vfs_getxattr(mnt_userns, d, kname, kvalue, size);
+ error = vfs_getxattr(mnt_userns, d, name, kvalue, size);
if (error > 0) {
- if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
- (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
+ if ((strcmp(name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
+ (strcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
posix_acl_fix_xattr_to_user(mnt_userns, kvalue, error);
if (size && copy_to_user(value, kvalue, error))
error = -EFAULT;
@@ -706,6 +699,22 @@ getxattr(struct user_namespace *mnt_userns, struct dentry *d,
return error;
}
+static ssize_t
+getxattr(struct user_namespace *mnt_userns, struct dentry *d,
+ const char __user *name, void __user *value, size_t size)
+{
+ ssize_t error;
+ char kname[XATTR_NAME_MAX + 1];
+
+ error = strncpy_from_user(kname, name, sizeof(kname));
+ if (error == 0 || error == sizeof(kname))
+ error = -ERANGE;
+ if (error < 0)
+ return error;
+
+ return do_getxattr(mnt_userns, d, kname, value, size);
+}
+
static ssize_t path_getxattr(const char __user *pathname,
const char __user *name, void __user *value,
size_t size, unsigned int lookup_flags)
--
2.30.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 4/5] io_uring: add fsetxattr and setxattr support
2021-11-29 22:12 [PATCH v1 0/5] io_uring: add xattr support Stefan Roesch
` (2 preceding siblings ...)
2021-11-29 22:12 ` [PATCH v1 3/5] fs: split off the vfs_getxattr from getxattr Stefan Roesch
@ 2021-11-29 22:12 ` Stefan Roesch
2021-11-29 22:12 ` [PATCH v1 5/5] io_uring: add fgetxattr and getxattr support Stefan Roesch
2021-11-30 1:08 ` [PATCH v1 0/5] io_uring: add xattr support Clay Harris
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Roesch @ 2021-11-29 22:12 UTC (permalink / raw)
To: io-uring, linux-fsdevel; +Cc: shr
Summary:
This adds support to io_uring for the following API's:
- fsetxattr
- setxattr
Signed-off-by: Stefan Roesch <[email protected]>
---
fs/io_uring.c | 173 ++++++++++++++++++++++++++++++++++
include/uapi/linux/io_uring.h | 6 +-
2 files changed, 178 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 568729677e25..4a18431e13a3 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -81,6 +81,7 @@
#include <linux/tracehook.h>
#include <linux/audit.h>
#include <linux/security.h>
+#include <linux/xattr.h>
#define CREATE_TRACE_POINTS
#include <trace/events/io_uring.h>
@@ -717,6 +718,13 @@ struct io_async_rw {
struct wait_page_queue wpq;
};
+struct io_xattr {
+ struct file *file;
+ struct xattr_ctx ctx;
+ void *value;
+ struct filename *filename;
+};
+
enum {
REQ_F_FIXED_FILE_BIT = IOSQE_FIXED_FILE_BIT,
REQ_F_IO_DRAIN_BIT = IOSQE_IO_DRAIN_BIT,
@@ -856,6 +864,7 @@ struct io_kiocb {
struct io_mkdir mkdir;
struct io_symlink symlink;
struct io_hardlink hardlink;
+ struct io_xattr xattr;
};
u8 opcode;
@@ -1105,6 +1114,10 @@ static const struct io_op_def io_op_defs[] = {
[IORING_OP_MKDIRAT] = {},
[IORING_OP_SYMLINKAT] = {},
[IORING_OP_LINKAT] = {},
+ [IORING_OP_FSETXATTR] = {
+ .needs_file = 1
+ },
+ [IORING_OP_SETXATTR] = {},
};
/* requests with any of those set should undergo io_disarm_next() */
@@ -3818,6 +3831,146 @@ static int io_renameat(struct io_kiocb *req, unsigned int issue_flags)
return 0;
}
+static int __io_setxattr_prep(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe,
+ struct user_namespace *user_ns)
+{
+ struct io_xattr *ix = &req->xattr;
+ const char __user *name;
+ void *ret;
+
+ if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
+ return -EINVAL;
+ if (unlikely(sqe->ioprio))
+ return -EINVAL;
+ if (unlikely(req->flags & REQ_F_FIXED_FILE))
+ return -EBADF;
+
+ ix->filename = NULL;
+ name = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ ix->ctx.value = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+ ix->ctx.size = READ_ONCE(sqe->len);
+ ix->ctx.flags = READ_ONCE(sqe->xattr_flags);
+
+ ix->ctx.name = kmalloc(XATTR_NAME_MAX + 1, GFP_KERNEL);
+ if (!ix->ctx.name)
+ return -ENOMEM;
+ ix->ctx.name_sz = XATTR_NAME_MAX + 1;
+
+ ret = setxattr_setup(user_ns, name, &ix->ctx);
+ if (IS_ERR(ret)) {
+ kfree(ix->ctx.name);
+ return PTR_ERR(ret);
+ }
+
+ ix->value = ret;
+ req->flags |= REQ_F_NEED_CLEANUP;
+ return 0;
+}
+
+static int io_setxattr_prep(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe)
+{
+ struct io_xattr *ix = &req->xattr;
+ const char __user *path;
+ int ret;
+
+ ret = __io_setxattr_prep(req, sqe, current_user_ns());
+ if (ret)
+ return ret;
+
+ path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
+
+ ix->filename = getname_flags(path, LOOKUP_FOLLOW, NULL);
+ if (IS_ERR(ix->filename)) {
+ ret = PTR_ERR(ix->filename);
+ ix->filename = NULL;
+ }
+
+ return ret;
+}
+
+static int io_fsetxattr_prep(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe)
+{
+ if (!req->file)
+ return -EBADF;
+
+ return __io_setxattr_prep(req, sqe, file_mnt_user_ns(req->file));
+}
+
+static int __io_setxattr(struct io_kiocb *req, unsigned int issue_flags,
+ struct path *path)
+{
+ struct io_xattr *ix = &req->xattr;
+ int ret;
+
+ ret = mnt_want_write(path->mnt);
+ if (!ret) {
+ ret = vfs_setxattr(mnt_user_ns(path->mnt), path->dentry,
+ ix->ctx.name, ix->value, ix->ctx.size,
+ ix->ctx.flags);
+ mnt_drop_write(path->mnt);
+ }
+
+ return ret;
+}
+
+static int io_fsetxattr(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_xattr *ix = &req->xattr;
+ int ret;
+
+ if (issue_flags & IO_URING_F_NONBLOCK)
+ return -EAGAIN;
+
+ ret = __io_setxattr(req, issue_flags, &req->file->f_path);
+
+ req->flags &= ~REQ_F_NEED_CLEANUP;
+ kfree(ix->ctx.name);
+
+ if (ix->value)
+ kvfree(ix->value);
+ if (ret < 0)
+ req_set_fail(req);
+
+ io_req_complete(req, ret);
+ return 0;
+}
+
+static int io_setxattr(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_xattr *ix = &req->xattr;
+ unsigned int lookup_flags = LOOKUP_FOLLOW;
+ struct path path;
+ int ret;
+
+ if (issue_flags & IO_URING_F_NONBLOCK)
+ return -EAGAIN;
+
+retry:
+ ret = user_path_at_empty(AT_FDCWD, ix->filename, lookup_flags, &path);
+ if (!ret) {
+ ret = __io_setxattr(req, issue_flags, &path);
+ path_put(&path);
+ if (retry_estale(ret, lookup_flags)) {
+ lookup_flags |= LOOKUP_REVAL;
+ goto retry;
+ }
+ }
+
+ req->flags &= ~REQ_F_NEED_CLEANUP;
+ kfree(ix->ctx.name);
+
+ if (ix->value)
+ kvfree(ix->value);
+ if (ret < 0)
+ req_set_fail(req);
+
+ io_req_complete(req, ret);
+ return 0;
+}
+
static int io_unlinkat_prep(struct io_kiocb *req,
const struct io_uring_sqe *sqe)
{
@@ -6531,6 +6684,10 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return io_symlinkat_prep(req, sqe);
case IORING_OP_LINKAT:
return io_linkat_prep(req, sqe);
+ case IORING_OP_FSETXATTR:
+ return io_fsetxattr_prep(req, sqe);
+ case IORING_OP_SETXATTR:
+ return io_setxattr_prep(req, sqe);
}
printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n",
@@ -6674,6 +6831,15 @@ static void io_clean_op(struct io_kiocb *req)
putname(req->hardlink.oldpath);
putname(req->hardlink.newpath);
break;
+ case IORING_OP_SETXATTR:
+ if (req->xattr.filename)
+ putname(req->xattr.filename);
+ fallthrough;
+ case IORING_OP_FSETXATTR:
+ kfree(req->xattr.ctx.name);
+ if (req->xattr.value)
+ kvfree(req->xattr.value);
+ break;
}
}
if ((req->flags & REQ_F_POLLED) && req->apoll) {
@@ -6816,6 +6982,12 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
case IORING_OP_LINKAT:
ret = io_linkat(req, issue_flags);
break;
+ case IORING_OP_FSETXATTR:
+ ret = io_fsetxattr(req, issue_flags);
+ break;
+ case IORING_OP_SETXATTR:
+ ret = io_setxattr(req, issue_flags);
+ break;
default:
ret = -EINVAL;
break;
@@ -11183,6 +11355,7 @@ static int __init io_uring_init(void)
BUILD_BUG_SQE_ELEM(42, __u16, personality);
BUILD_BUG_SQE_ELEM(44, __s32, splice_fd_in);
BUILD_BUG_SQE_ELEM(44, __u32, file_index);
+ BUILD_BUG_SQE_ELEM(48, __u64, addr3);
BUILD_BUG_ON(sizeof(struct io_uring_files_update) !=
sizeof(struct io_uring_rsrc_update));
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 787f491f0d2a..dbf473900da2 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -45,6 +45,7 @@ struct io_uring_sqe {
__u32 rename_flags;
__u32 unlink_flags;
__u32 hardlink_flags;
+ __u32 xattr_flags;
};
__u64 user_data; /* data to be passed back at completion time */
/* pack this to avoid bogus arm OABI complaints */
@@ -60,7 +61,8 @@ struct io_uring_sqe {
__s32 splice_fd_in;
__u32 file_index;
};
- __u64 __pad2[2];
+ __u64 addr3;
+ __u64 __pad2[1];
};
enum {
@@ -143,6 +145,8 @@ enum {
IORING_OP_MKDIRAT,
IORING_OP_SYMLINKAT,
IORING_OP_LINKAT,
+ IORING_OP_FSETXATTR,
+ IORING_OP_SETXATTR,
/* this goes last, obviously */
IORING_OP_LAST,
--
2.30.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 5/5] io_uring: add fgetxattr and getxattr support
2021-11-29 22:12 [PATCH v1 0/5] io_uring: add xattr support Stefan Roesch
` (3 preceding siblings ...)
2021-11-29 22:12 ` [PATCH v1 4/5] io_uring: add fsetxattr and setxattr support Stefan Roesch
@ 2021-11-29 22:12 ` Stefan Roesch
2021-11-30 1:08 ` [PATCH v1 0/5] io_uring: add xattr support Clay Harris
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Roesch @ 2021-11-29 22:12 UTC (permalink / raw)
To: io-uring, linux-fsdevel; +Cc: shr
Summary:
This adds support to io_uring for the following API's:
- fgetxattr
- getxattr
Signed-off-by: Stefan Roesch <[email protected]>
---
fs/io_uring.c | 152 ++++++++++++++++++++++++++++++++++
include/uapi/linux/io_uring.h | 2 +
2 files changed, 154 insertions(+)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 4a18431e13a3..265024204a59 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1114,6 +1114,10 @@ static const struct io_op_def io_op_defs[] = {
[IORING_OP_MKDIRAT] = {},
[IORING_OP_SYMLINKAT] = {},
[IORING_OP_LINKAT] = {},
+ [IORING_OP_FGETXATTR] = {
+ .needs_file = 1
+ },
+ [IORING_OP_GETXATTR] = {},
[IORING_OP_FSETXATTR] = {
.needs_file = 1
},
@@ -3831,6 +3835,135 @@ static int io_renameat(struct io_kiocb *req, unsigned int issue_flags)
return 0;
}
+static int __io_getxattr_prep(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe)
+{
+ struct io_xattr *ix = &req->xattr;
+ const char __user *name;
+ int ret;
+
+ if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
+ return -EINVAL;
+ if (unlikely(sqe->ioprio))
+ return -EINVAL;
+ if (unlikely(req->flags & REQ_F_FIXED_FILE))
+ return -EBADF;
+
+ ix->filename = NULL;
+ ix->value = NULL;
+ name = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ ix->ctx.value = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+ ix->ctx.size = READ_ONCE(sqe->len);
+ ix->ctx.flags = READ_ONCE(sqe->xattr_flags);
+
+ if (ix->ctx.flags)
+ return -EINVAL;
+
+ ix->ctx.name = kmalloc(XATTR_NAME_MAX + 1, GFP_KERNEL);
+ if (!ix->ctx.name)
+ return -ENOMEM;
+
+ ret = strncpy_from_user(ix->ctx.name, name, XATTR_NAME_MAX + 1);
+ if (!ret || ret == XATTR_NAME_MAX + 1)
+ ret = -ERANGE;
+ if (ret < 0) {
+ kfree(ix->ctx.name);
+ return ret;
+ }
+
+ req->flags |= REQ_F_NEED_CLEANUP;
+ return 0;
+}
+
+static int io_fgetxattr_prep(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe)
+{
+ if (!req->file)
+ return -EBADF;
+
+ return __io_getxattr_prep(req, sqe);
+}
+
+static int io_getxattr_prep(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe)
+{
+ struct io_xattr *ix = &req->xattr;
+ const char __user *path;
+ int ret;
+
+ ret = __io_getxattr_prep(req, sqe);
+ if (ret)
+ return ret;
+
+ path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
+
+ ix->filename = getname_flags(path, LOOKUP_FOLLOW, NULL);
+ if (IS_ERR(ix->filename)) {
+ ret = PTR_ERR(ix->filename);
+ ix->filename = NULL;
+ }
+
+ return ret;
+}
+
+static int io_fgetxattr(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_xattr *ix = &req->xattr;
+ int ret;
+
+ if (issue_flags & IO_URING_F_NONBLOCK)
+ return -EAGAIN;
+
+ ret = do_getxattr(mnt_user_ns(req->file->f_path.mnt),
+ req->file->f_path.dentry,
+ ix->ctx.name,
+ (void __user *)ix->ctx.value,
+ ix->ctx.size);
+
+ req->flags &= ~REQ_F_NEED_CLEANUP;
+ kfree(ix->ctx.name);
+ if (ret < 0)
+ req_set_fail(req);
+
+ io_req_complete(req, ret);
+ return 0;
+}
+
+static int io_getxattr(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_xattr *ix = &req->xattr;
+ unsigned int lookup_flags = LOOKUP_FOLLOW;
+ struct path path;
+ int ret;
+
+ if (issue_flags & IO_URING_F_NONBLOCK)
+ return -EAGAIN;
+
+retry:
+ ret = user_path_at_empty(AT_FDCWD, ix->filename, lookup_flags, &path);
+ if (!ret) {
+ ret = do_getxattr(mnt_user_ns(path.mnt),
+ path.dentry,
+ ix->ctx.name,
+ (void __user *)ix->ctx.value,
+ ix->ctx.size);
+
+ path_put(&path);
+ if (retry_estale(ret, lookup_flags)) {
+ lookup_flags |= LOOKUP_REVAL;
+ goto retry;
+ }
+ }
+
+ req->flags &= ~REQ_F_NEED_CLEANUP;
+ kfree(ix->ctx.name);
+ if (ret < 0)
+ req_set_fail(req);
+
+ io_req_complete(req, ret);
+ return 0;
+}
+
static int __io_setxattr_prep(struct io_kiocb *req,
const struct io_uring_sqe *sqe,
struct user_namespace *user_ns)
@@ -6684,6 +6817,10 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return io_symlinkat_prep(req, sqe);
case IORING_OP_LINKAT:
return io_linkat_prep(req, sqe);
+ case IORING_OP_FGETXATTR:
+ return io_fgetxattr_prep(req, sqe);
+ case IORING_OP_GETXATTR:
+ return io_getxattr_prep(req, sqe);
case IORING_OP_FSETXATTR:
return io_fsetxattr_prep(req, sqe);
case IORING_OP_SETXATTR:
@@ -6831,6 +6968,15 @@ static void io_clean_op(struct io_kiocb *req)
putname(req->hardlink.oldpath);
putname(req->hardlink.newpath);
break;
+
+ case IORING_OP_GETXATTR:
+ if (req->xattr.filename)
+ putname(req->xattr.filename);
+ fallthrough;
+ case IORING_OP_FGETXATTR:
+ kfree(req->xattr.ctx.name);
+ break;
+
case IORING_OP_SETXATTR:
if (req->xattr.filename)
putname(req->xattr.filename);
@@ -6982,6 +7128,12 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
case IORING_OP_LINKAT:
ret = io_linkat(req, issue_flags);
break;
+ case IORING_OP_FGETXATTR:
+ ret = io_fgetxattr(req, issue_flags);
+ break;
+ case IORING_OP_GETXATTR:
+ ret = io_getxattr(req, issue_flags);
+ break;
case IORING_OP_FSETXATTR:
ret = io_fsetxattr(req, issue_flags);
break;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index dbf473900da2..cd9160272308 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -145,7 +145,9 @@ enum {
IORING_OP_MKDIRAT,
IORING_OP_SYMLINKAT,
IORING_OP_LINKAT,
+ IORING_OP_FGETXATTR,
IORING_OP_FSETXATTR,
+ IORING_OP_GETXATTR,
IORING_OP_SETXATTR,
/* this goes last, obviously */
--
2.30.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-29 22:12 [PATCH v1 0/5] io_uring: add xattr support Stefan Roesch
` (4 preceding siblings ...)
2021-11-29 22:12 ` [PATCH v1 5/5] io_uring: add fgetxattr and getxattr support Stefan Roesch
@ 2021-11-30 1:08 ` Clay Harris
2021-11-30 3:16 ` Andreas Dilger
2021-12-01 6:07 ` Stefan Roesch
5 siblings, 2 replies; 21+ messages in thread
From: Clay Harris @ 2021-11-30 1:08 UTC (permalink / raw)
To: Stefan Roesch; +Cc: io-uring, linux-fsdevel
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);
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.
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 <[email protected]>
> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
> --
> 2.30.2
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-30 1:08 ` [PATCH v1 0/5] io_uring: add xattr support Clay Harris
@ 2021-11-30 3:16 ` Andreas Dilger
2021-11-30 6:37 ` Clay Harris
` (2 more replies)
2021-12-01 6:07 ` Stefan Roesch
1 sibling, 3 replies; 21+ messages in thread
From: Andreas Dilger @ 2021-11-30 3:16 UTC (permalink / raw)
To: Clay Harris; +Cc: Stefan Roesch, io-uring, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 3273 bytes --]
> On Nov 29, 2021, at 6:08 PM, Clay Harris <[email protected]> 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.
> 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 <[email protected]>
>> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
>> --
>> 2.30.2
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-30 3:16 ` Andreas Dilger
@ 2021-11-30 6:37 ` Clay Harris
2021-11-30 6:53 ` Clay Harris
2021-11-30 7:19 ` Dave Chinner
2021-12-01 6:16 ` Stefan Roesch
2 siblings, 1 reply; 21+ messages in thread
From: Clay Harris @ 2021-11-30 6:37 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Stefan Roesch, io-uring, linux-fsdevel
On Mon, Nov 29 2021 at 20:16:02 -0700, Andreas Dilger quoth thus:
>
> > On Nov 29, 2021, at 6:08 PM, Clay Harris <[email protected]> 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/
> >
> >> 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 <[email protected]>
> >> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
> >> --
> >> 2.30.2
>
>
> Cheers, Andreas
>
>
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-30 6:37 ` Clay Harris
@ 2021-11-30 6:53 ` Clay Harris
2021-11-30 11:40 ` Clay Harris
0 siblings, 1 reply; 21+ messages in thread
From: Clay Harris @ 2021-11-30 6:53 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Stefan Roesch, io-uring, linux-fsdevel
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 <[email protected]> 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 <[email protected]>
> > >> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
> > >> --
> > >> 2.30.2
> >
> >
> > Cheers, Andreas
> >
> >
> >
> >
> >
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-30 6:53 ` Clay Harris
@ 2021-11-30 11:40 ` Clay Harris
0 siblings, 0 replies; 21+ messages in thread
From: Clay Harris @ 2021-11-30 11:40 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Stefan Roesch, io-uring, linux-fsdevel
On Tue, Nov 30 2021 at 00:53:45 -0600, Clay Harris quoth thus:
> 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 <[email protected]> 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.
One more reason, it would be very desirable if io_uring called a
*etxattrat-like interface, is that the old f*etxattr calls require an fd
open for reading (fget*) or writing (fset*). So, you're out of luck if
you have an execute-only file or just an O_PATH descriptor! In those
cases, you're forced to use a pathname for every call. Not very efficient
for people who choose to use the highly optimized io_uring interface.
> > > 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 <[email protected]>
> > > >> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
> > > >> --
> > > >> 2.30.2
> > >
> > >
> > > Cheers, Andreas
> > >
> > >
> > >
> > >
> > >
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-30 3:16 ` Andreas Dilger
2021-11-30 6:37 ` Clay Harris
@ 2021-11-30 7:19 ` Dave Chinner
2021-12-01 6:16 ` Stefan Roesch
2 siblings, 0 replies; 21+ messages in thread
From: Dave Chinner @ 2021-11-30 7:19 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Clay Harris, Stefan Roesch, io-uring, linux-fsdevel
On Mon, Nov 29, 2021 at 08:16:02PM -0700, Andreas Dilger wrote:
>
> > On Nov 29, 2021, at 6:08 PM, Clay Harris <[email protected]> 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.
Been around for since the mid 1990s IIRC. XFS brought them to Linux
from Irix and they are used by xfsdump/restore via libhandle. API
documented here:
$ man 3 attr_multi
And they are implemented through XFS ioctls.
Cheers,
Dave.
--
Dave Chinner
[email protected]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-30 3:16 ` Andreas Dilger
2021-11-30 6:37 ` Clay Harris
2021-11-30 7:19 ` Dave Chinner
@ 2021-12-01 6:16 ` Stefan Roesch
2 siblings, 0 replies; 21+ messages in thread
From: Stefan Roesch @ 2021-12-01 6:16 UTC (permalink / raw)
To: Andreas Dilger, Clay Harris; +Cc: io-uring, linux-fsdevel
On 11/29/21 7:16 PM, Andreas Dilger wrote:
>
>> On Nov 29, 2021, at 6:08 PM, Clay Harris <[email protected]> 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 <[email protected]>
>>> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
>>> --
>>> 2.30.2
>
>
> Cheers, Andreas
>
>
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-11-30 1:08 ` [PATCH v1 0/5] io_uring: add xattr support Clay Harris
2021-11-30 3:16 ` Andreas Dilger
@ 2021-12-01 6:07 ` Stefan Roesch
2021-12-01 7:46 ` Clay Harris
2021-12-01 12:19 ` Stefan Metzmacher
1 sibling, 2 replies; 21+ messages in thread
From: Stefan Roesch @ 2021-12-01 6:07 UTC (permalink / raw)
To: Clay Harris; +Cc: io-uring, linux-fsdevel
On 11/29/21 5:08 PM, Clay Harris 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);
>
> 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.
>
> 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.
Clay,
while we can reduce the number of calls to 2, providing 4 calls will
ease the adoption of the interface.
If you look at the userspace interface in liburing, you can see the
following function signature:
static inline void io_uring_prep_fgetxattr(struct io_uring_sqe *sqe,
int fd,
const char *name,
const char *value,
size_t len)
This is very similar to what you proposed.
>
>> 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 <[email protected]>
>> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
>> --
>> 2.30.2
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-12-01 6:07 ` Stefan Roesch
@ 2021-12-01 7:46 ` Clay Harris
2021-12-01 13:14 ` Christian Brauner
2021-12-01 12:19 ` Stefan Metzmacher
1 sibling, 1 reply; 21+ messages in thread
From: Clay Harris @ 2021-12-01 7:46 UTC (permalink / raw)
To: Stefan Roesch; +Cc: io-uring, linux-fsdevel
On Tue, Nov 30 2021 at 22:07:47 -0800, Stefan Roesch quoth thus:
>
>
> On 11/29/21 5:08 PM, Clay Harris 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);
> >
> > 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.
> >
> > 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.
>
I may have become a little over-excited when I saw someone was thinking
about new code associated with these interfaces. It's just that, to be
very kind, the existing interfaces have so much room for improvement.
I'm aware that changes in this area can be a non-trivial amount of
work, due to specific xattr keys being handled by different security
module hooks.
> Clay,
>
> while we can reduce the number of calls to 2, providing 4 calls will
> ease the adoption of the interface.
Well, there's removexattr(), but who's counting?
I believe people use the other *at() interfaces without ever looking
back at the old calls and that there is little point in io_uring reproducing
all of the old baggage.
> If you look at the userspace interface in liburing, you can see the
> following function signature:
>
> static inline void io_uring_prep_fgetxattr(struct io_uring_sqe *sqe,
> int fd,
> const char *name,
> const char *value,
> size_t len)
>
> This is very similar to what you proposed.
Even though these functions desperately need updating, and as super nice
as it would be, I don't expect you to implement getxattrat() and setxattrat().
If I were to name a single thing that would most increase the usability of
these functions, it would be:
Make the fXetxattr() functions (at least the io_uring versions)
work with an O_PATH descriptor.
That one thing would at least provide most of the desired functionality at
the cost of an extra openat() call.
>
> >
> >> 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 <[email protected]>
> >> base-commit: c2626d30f312afc341158e07bf088f5a23b4eeeb
> >> --
> >> 2.30.2
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-12-01 7:46 ` Clay Harris
@ 2021-12-01 13:14 ` Christian Brauner
0 siblings, 0 replies; 21+ messages in thread
From: Christian Brauner @ 2021-12-01 13:14 UTC (permalink / raw)
To: Clay Harris; +Cc: Stefan Roesch, io-uring, linux-fsdevel
On Wed, Dec 01, 2021 at 01:46:21AM -0600, Clay Harris wrote:
> On Tue, Nov 30 2021 at 22:07:47 -0800, Stefan Roesch quoth thus:
>
> >
> >
> > On 11/29/21 5:08 PM, Clay Harris 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);
> > >
> > > 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.
> > >
> > > 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.
> >
>
> I may have become a little over-excited when I saw someone was thinking
> about new code associated with these interfaces. It's just that, to be
> very kind, the existing interfaces have so much room for improvement.
> I'm aware that changes in this area can be a non-trivial amount of
> work, due to specific xattr keys being handled by different security
> module hooks.
>
> > Clay,
> >
> > while we can reduce the number of calls to 2, providing 4 calls will
> > ease the adoption of the interface.
>
> Well, there's removexattr(), but who's counting?
> I believe people use the other *at() interfaces without ever looking
> back at the old calls and that there is little point in io_uring reproducing
> all of the old baggage.
>
> > If you look at the userspace interface in liburing, you can see the
> > following function signature:
> >
> > static inline void io_uring_prep_fgetxattr(struct io_uring_sqe *sqe,
> > int fd,
> > const char *name,
> > const char *value,
> > size_t len)
> >
> > This is very similar to what you proposed.
>
> Even though these functions desperately need updating, and as super nice
This code could use some serious cleanup as it is super hard to follow
right now imho. It often gives the impression of forming loops when
following callchains down into the filesystem. None of this is by
design of course. I just happened to grow that way, I guess.
However, for maintenance this is quite painful. I also don't like that
the relationship between xattr and acls and the .set_acl inode methods
is rather opaque in the code. I have a vague plan to cleanup some of
that since I had to mess with this code not too long ago.
But that'll be a bigger chunk of work.
Christian
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-12-01 6:07 ` Stefan Roesch
2021-12-01 7:46 ` Clay Harris
@ 2021-12-01 12:19 ` Stefan Metzmacher
2021-12-01 19:52 ` Clay Harris
2021-12-03 17:58 ` Stefan Roesch
1 sibling, 2 replies; 21+ messages in thread
From: Stefan Metzmacher @ 2021-12-01 12:19 UTC (permalink / raw)
To: Stefan Roesch, Clay Harris; +Cc: io-uring, linux-fsdevel
Hi Stefan,
> On 11/29/21 5:08 PM, Clay Harris 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);
>>
>> 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.
>>
>> 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.
>
> Clay,
>
> while we can reduce the number of calls to 2, providing 4 calls will
> ease the adoption of the interface.
>
> If you look at the userspace interface in liburing, you can see the
> following function signature:
>
> static inline void io_uring_prep_fgetxattr(struct io_uring_sqe *sqe,
> int fd,
> const char *name,
> const char *value,
> size_t len)
>
> This is very similar to what you proposed.
What's with lsetxattr and lgetxattr, why are they missing.
I'd assume that even 6 helper functions in liburing would be able
to use just 2 low level iouring opcodes.
*listxattr is also missing, are there plans for them?
metze
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-12-01 12:19 ` Stefan Metzmacher
@ 2021-12-01 19:52 ` Clay Harris
2021-12-01 20:05 ` Andreas Dilger
2021-12-03 17:58 ` Stefan Roesch
1 sibling, 1 reply; 21+ messages in thread
From: Clay Harris @ 2021-12-01 19:52 UTC (permalink / raw)
To: Stefan Metzmacher; +Cc: Stefan Roesch, io-uring, linux-fsdevel
On Wed, Dec 01 2021 at 13:19:03 +0100, Stefan Metzmacher quoth thus:
> Hi Stefan,
>
> > On 11/29/21 5:08 PM, Clay Harris 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);
> >>
> >> 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.
> >>
> >> 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.
> >
> > Clay,
> >
> > while we can reduce the number of calls to 2, providing 4 calls will
> > ease the adoption of the interface.
> >
> > If you look at the userspace interface in liburing, you can see the
> > following function signature:
> >
> > static inline void io_uring_prep_fgetxattr(struct io_uring_sqe *sqe,
> > int fd,
> > const char *name,
> > const char *value,
> > size_t len)
> >
> > This is very similar to what you proposed.
>
> What's with lsetxattr and lgetxattr, why are they missing.
Do any filesystems even support xattrs on symbolic links?
> I'd assume that even 6 helper functions in liburing would be able
> to use just 2 low level iouring opcodes.
>
> *listxattr is also missing, are there plans for them?
>
> metze
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-12-01 19:52 ` Clay Harris
@ 2021-12-01 20:05 ` Andreas Dilger
0 siblings, 0 replies; 21+ messages in thread
From: Andreas Dilger @ 2021-12-01 20:05 UTC (permalink / raw)
To: Clay Harris; +Cc: Stefan Metzmacher, Stefan Roesch, io-uring, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
On Dec 1, 2021, at 12:52 PM, Clay Harris <[email protected]> wrote:
>
> On Wed, Dec 01 2021 at 13:19:03 +0100, Stefan Metzmacher quoth thus:
>>
>> What's with lsetxattr and lgetxattr, why are they missing.
> Do any filesystems even support xattrs on symbolic links?
Yes, ext4 does. There are definitely security xattrs on symlinks that
need to be accessed and changed.
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/5] io_uring: add xattr support
2021-12-01 12:19 ` Stefan Metzmacher
2021-12-01 19:52 ` Clay Harris
@ 2021-12-03 17:58 ` Stefan Roesch
1 sibling, 0 replies; 21+ messages in thread
From: Stefan Roesch @ 2021-12-03 17:58 UTC (permalink / raw)
To: Stefan Metzmacher, Clay Harris; +Cc: io-uring, linux-fsdevel
Hi Stefan,
On 12/1/21 4:19 AM, Stefan Metzmacher wrote:
> Hi Stefan,
>
>> On 11/29/21 5:08 PM, Clay Harris 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);
>>>
>>> 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.
>>>
>>> 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.
>>
>> Clay,
>>
>> while we can reduce the number of calls to 2, providing 4 calls will
>> ease the adoption of the interface.
>>
>> If you look at the userspace interface in liburing, you can see the
>> following function signature:
>>
>> static inline void io_uring_prep_fgetxattr(struct io_uring_sqe *sqe,
>> int fd,
>> const char *name,
>> const char *value,
>> size_t len)
>>
>> This is very similar to what you proposed.
>
> What's with lsetxattr and lgetxattr, why are they missing.
>
> I'd assume that even 6 helper functions in liburing would be able
> to use just 2 low level iouring opcodes.
>
I'm open to also adding lsetxattr and lgetxattr. Do you have a use case in mind?
> *listxattr is also missing, are there plans for them?
>
*listxattr is currently not planned.
> metze
>
^ permalink raw reply [flat|nested] 21+ messages in thread