Re: [PATCH 6/6] btrfs: add io_uring interface for encoded reads

[Pavel Begunkov <asml.silence@gmail.com>]

On 8/23/24 17:27, Mark Harmstone wrote:
> Adds an io_uring interface for asynchronous encoded reads, using the
> same interface as for the ioctl. To use this you would use an SQE opcode
> of IORING_OP_URING_CMD, the cmd_op would be BTRFS_IOC_ENCODED_READ, and
> addr would point to the userspace address of the
> btrfs_ioctl_encoded_io_args struct. As with the ioctl, you need to have
> CAP_SYS_ADMIN for this to work.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
...
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 1bd4c74e8c51..e4458168c340 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -34,6 +34,7 @@
>   #include <linux/iomap.h>
>   #include <asm/unaligned.h>
>   #include <linux/fsverity.h>
> +#include <linux/io_uring/cmd.h>
>   #include "misc.h"
>   #include "ctree.h"
>   #include "disk-io.h"
> @@ -9078,7 +9079,7 @@ static ssize_t btrfs_encoded_read_inline(
>   	return ret;
>   }
>   
> -static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
> +static void btrfs_encoded_read_ioctl_endio(struct btrfs_bio *bbio)
>   {
>   	struct btrfs_encoded_read_private *priv = bbio->private;
>   
> @@ -9098,6 +9099,47 @@ static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
>   	bio_put(&bbio->bio);
>   }
>   
> +static inline struct btrfs_encoded_read_private *btrfs_uring_encoded_read_pdu(
> +		struct io_uring_cmd *cmd)
> +{
> +	return *(struct btrfs_encoded_read_private **)cmd->pdu;
> +}
> +static void btrfs_finish_uring_encoded_read(struct io_uring_cmd *cmd,
> +					    unsigned int issue_flags)
> +{
> +	struct btrfs_encoded_read_private *priv;
> +	ssize_t ret;
> +
> +	priv = btrfs_uring_encoded_read_pdu(cmd);
> +	ret = btrfs_encoded_read_finish(priv, -EIOCBQUEUED);

tw callback -> btrfs_encoded_read_finish() -> copy_to_user()

That's usually fine except cases when the task and/or io_uring are dying
and the callback executes not from a user task context. Same problem as
with fuse, we can pass a flag from io_uring into the callback telling
btrfs that it should terminate the request and not rely on mm or any
other task related pieces.


> +
> +	io_uring_cmd_done(priv->cmd, ret, 0, priv->issue_flags);
> +
> +	kfree(priv);
> +}
> +

...

-- 
Pavel Begunkov