btrfs is the only user of struct io_uring_cmd_data and its op_data
field. Switch its ->uring_cmd() implementations to store the struct
btrfs_uring_encoded_data * in the struct io_btrfs_cmd, overlayed with
io_uring_cmd's pdu field. This avoids having to touch another cache line
to access the struct btrfs_uring_encoded_data *, and allows op_data and
struct io_uring_cmd_data to be removed.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
fs/btrfs/ioctl.c | 38 +++++++++++++++++++++++++++-----------
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ff15160e2581..6183729c93a1 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4627,10 +4627,17 @@ static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp, bool
add_wchar(current, ret);
inc_syscw(current);
return ret;
}
+struct btrfs_uring_encoded_data {
+ struct btrfs_ioctl_encoded_io_args args;
+ struct iovec iovstack[UIO_FASTIOV];
+ struct iovec *iov;
+ struct iov_iter iter;
+};
+
/*
* Context that's attached to an encoded read io_uring command, in cmd->pdu. It
* contains the fields in btrfs_uring_read_extent that are necessary to finish
* off and cleanup the I/O in btrfs_uring_read_finished.
*/
@@ -4648,10 +4655,11 @@ struct btrfs_uring_priv {
int err;
bool compressed;
};
struct io_btrfs_cmd {
+ struct btrfs_uring_encoded_data *data;
struct btrfs_uring_priv *priv;
};
static void btrfs_uring_read_finished(struct io_uring_cmd *cmd, unsigned int issue_flags)
{
@@ -4706,10 +4714,11 @@ static void btrfs_uring_read_finished(struct io_uring_cmd *cmd, unsigned int iss
__free_page(priv->pages[index]);
kfree(priv->pages);
kfree(priv->iov);
kfree(priv);
+ kfree(bc->data);
}
void btrfs_uring_read_extent_endio(void *ctx, int err)
{
struct btrfs_uring_priv *priv = ctx;
@@ -4789,17 +4798,10 @@ static int btrfs_uring_read_extent(struct kiocb *iocb, struct iov_iter *iter,
btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
kfree(priv);
return ret;
}
-struct btrfs_uring_encoded_data {
- struct btrfs_ioctl_encoded_io_args args;
- struct iovec iovstack[UIO_FASTIOV];
- struct iovec *iov;
- struct iov_iter iter;
-};
-
static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue_flags)
{
size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args, flags);
size_t copy_end;
int ret;
@@ -4811,11 +4813,15 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
loff_t pos;
struct kiocb kiocb;
struct extent_state *cached_state = NULL;
u64 start, lockend;
void __user *sqe_addr;
- struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data;
+ struct io_btrfs_cmd *bc = io_uring_cmd_to_pdu(cmd, struct io_btrfs_cmd);
+ struct btrfs_uring_encoded_data *data = NULL;
+
+ if (cmd->flags & IORING_URING_CMD_REISSUE)
+ data = bc->data;
if (!capable(CAP_SYS_ADMIN)) {
ret = -EPERM;
goto out_acct;
}
@@ -4841,11 +4847,11 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
if (!data) {
ret = -ENOMEM;
goto out_acct;
}
- io_uring_cmd_get_async_data(cmd)->op_data = data;
+ bc->data = data;
if (issue_flags & IO_URING_F_COMPAT) {
#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
struct btrfs_ioctl_encoded_io_args_32 args32;
@@ -4939,21 +4945,28 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
out_acct:
if (ret > 0)
add_rchar(current, ret);
inc_syscr(current);
+ if (ret != -EIOCBQUEUED && ret != -EAGAIN)
+ kfree(data);
+
return ret;
}
static int btrfs_uring_encoded_write(struct io_uring_cmd *cmd, unsigned int issue_flags)
{
loff_t pos;
struct kiocb kiocb;
struct file *file;
ssize_t ret;
void __user *sqe_addr;
- struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data;
+ struct io_btrfs_cmd *bc = io_uring_cmd_to_pdu(cmd, struct io_btrfs_cmd);
+ struct btrfs_uring_encoded_data *data = NULL;
+
+ if (cmd->flags & IORING_URING_CMD_REISSUE)
+ data = bc->data;
if (!capable(CAP_SYS_ADMIN)) {
ret = -EPERM;
goto out_acct;
}
@@ -4971,11 +4984,11 @@ static int btrfs_uring_encoded_write(struct io_uring_cmd *cmd, unsigned int issu
if (!data) {
ret = -ENOMEM;
goto out_acct;
}
- io_uring_cmd_get_async_data(cmd)->op_data = data;
+ bc->data = data;
if (issue_flags & IO_URING_F_COMPAT) {
#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
struct btrfs_ioctl_encoded_io_args_32 args32;
@@ -5061,10 +5074,13 @@ static int btrfs_uring_encoded_write(struct io_uring_cmd *cmd, unsigned int issu
kfree(data->iov);
out_acct:
if (ret > 0)
add_wchar(current, ret);
inc_syscw(current);
+
+ if (ret != -EAGAIN)
+ kfree(data);
return ret;
}
int btrfs_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
{
--
2.45.2> @@ -4811,11 +4813,15 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue > loff_t pos; > struct kiocb kiocb; > struct extent_state *cached_state = NULL; > u64 start, lockend; > void __user *sqe_addr; > - struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data; > + struct io_btrfs_cmd *bc = io_uring_cmd_to_pdu(cmd, struct io_btrfs_cmd); > + struct btrfs_uring_encoded_data *data = NULL; > + > + if (cmd->flags & IORING_URING_CMD_REISSUE) > + data = bc->data; Can this be a btrfs io_btrfs_cmd specific flag? Doesn't seem like it would need to be io_uring wide. -- Jens Axboe
On Tue, Jul 1, 2025 at 3:06 PM Jens Axboe <axboe@kernel.dk> wrote: > > > @@ -4811,11 +4813,15 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue > > loff_t pos; > > struct kiocb kiocb; > > struct extent_state *cached_state = NULL; > > u64 start, lockend; > > void __user *sqe_addr; > > - struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data; > > + struct io_btrfs_cmd *bc = io_uring_cmd_to_pdu(cmd, struct io_btrfs_cmd); > > + struct btrfs_uring_encoded_data *data = NULL; > > + > > + if (cmd->flags & IORING_URING_CMD_REISSUE) > > + data = bc->data; > > Can this be a btrfs io_btrfs_cmd specific flag? Doesn't seem like it > would need to be io_uring wide. Maybe. But where are you thinking it would be stored? I don't think io_uring_cmd's pdu field would work because it's not initialized before the first call to ->uring_cmd(). That's the whole reason I needed to add a flag to tell whether this was the first call to ->uring_cmd() or a subsequent one. I also put the flag in the uring_cmd layer because that's where op_data was defined. Even though btrfs is the only current user of op_data, it seems like it was intended as a generic mechanism that other ->uring_cmd() implementations might want to use. It seems like the same argument would apply to this flag. Thoughts? Best, Caleb
On 7/2/25 1:51 PM, Caleb Sander Mateos wrote: > On Tue, Jul 1, 2025 at 3:06?PM Jens Axboe <axboe@kernel.dk> wrote: >> >>> @@ -4811,11 +4813,15 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue >>> loff_t pos; >>> struct kiocb kiocb; >>> struct extent_state *cached_state = NULL; >>> u64 start, lockend; >>> void __user *sqe_addr; >>> - struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data; >>> + struct io_btrfs_cmd *bc = io_uring_cmd_to_pdu(cmd, struct io_btrfs_cmd); >>> + struct btrfs_uring_encoded_data *data = NULL; >>> + >>> + if (cmd->flags & IORING_URING_CMD_REISSUE) >>> + data = bc->data; >> >> Can this be a btrfs io_btrfs_cmd specific flag? Doesn't seem like it >> would need to be io_uring wide. > > Maybe. But where are you thinking it would be stored? I don't think > io_uring_cmd's pdu field would work because it's not initialized > before the first call to ->uring_cmd(). That's the whole reason I > needed to add a flag to tell whether this was the first call to > ->uring_cmd() or a subsequent one. > I also put the flag in the uring_cmd layer because that's where > op_data was defined. Even though btrfs is the only current user of > op_data, it seems like it was intended as a generic mechanism that > other ->uring_cmd() implementations might want to use. It seems like > the same argument would apply to this flag. > Thoughts? It's probably fine as-is, it was just some quick reading of it. I'd like to stage this up so we can get it done for 6.17. Can you respind with the other minor comments addressed? And then we can attempt to work this out with the btrfs side. -- Jens Axboe
On Tue, Jul 8, 2025 at 2:17 PM Jens Axboe <axboe@kernel.dk> wrote: > > On 7/2/25 1:51 PM, Caleb Sander Mateos wrote: > > On Tue, Jul 1, 2025 at 3:06?PM Jens Axboe <axboe@kernel.dk> wrote: > >> > >>> @@ -4811,11 +4813,15 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue > >>> loff_t pos; > >>> struct kiocb kiocb; > >>> struct extent_state *cached_state = NULL; > >>> u64 start, lockend; > >>> void __user *sqe_addr; > >>> - struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data; > >>> + struct io_btrfs_cmd *bc = io_uring_cmd_to_pdu(cmd, struct io_btrfs_cmd); > >>> + struct btrfs_uring_encoded_data *data = NULL; > >>> + > >>> + if (cmd->flags & IORING_URING_CMD_REISSUE) > >>> + data = bc->data; > >> > >> Can this be a btrfs io_btrfs_cmd specific flag? Doesn't seem like it > >> would need to be io_uring wide. > > > > Maybe. But where are you thinking it would be stored? I don't think > > io_uring_cmd's pdu field would work because it's not initialized > > before the first call to ->uring_cmd(). That's the whole reason I > > needed to add a flag to tell whether this was the first call to > > ->uring_cmd() or a subsequent one. > > I also put the flag in the uring_cmd layer because that's where > > op_data was defined. Even though btrfs is the only current user of > > op_data, it seems like it was intended as a generic mechanism that > > other ->uring_cmd() implementations might want to use. It seems like > > the same argument would apply to this flag. > > Thoughts? > > It's probably fine as-is, it was just some quick reading of it. > > I'd like to stage this up so we can get it done for 6.17. Can you > respind with the other minor comments addressed? And then we can attempt > to work this out with the btrfs side. Sure, I can definitely incorporate the refactoring suggestion. Will try to resend the patch series today. Best, Caleb
© 2016 - 2025 Red Hat, Inc.