Re: [RFC PATCH 2/2] btrfs: ioctl: use registered buffer for IORING_URING_CMD_FIXED

[Sidong Yang <sidong.yang@furiosa.ai>]

On Tue, Mar 11, 2025 at 12:55:48PM +0000, Pavel Begunkov wrote:

Hi Pavel,

> On 3/11/25 11:40, Sidong Yang wrote:
> > This patch supports IORING_URING_CMD_FIXED flags in io-uring cmd. It
> > means that user provided buf_index in sqe that is registered before
> > submitting requests. In this patch, btrfs_uring_encoded_read() makes
> > iov_iter bvec type by checking the io-uring cmd flag. And there is
> > additional iou_vec field in btrfs_uring_priv for remaining bvecs
> > lifecycle.
> > 
> > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > ---
> >   fs/btrfs/ioctl.c | 26 +++++++++++++++++++++-----
> >   1 file changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index 6c18bad53cd3..586671eea622 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -4643,6 +4643,7 @@ struct btrfs_uring_priv {
> >   	struct page **pages;
> >   	unsigned long nr_pages;
> >   	struct kiocb iocb;
> > +	struct iou_vec iou_vec;
> 
> This structure should not be leaked out of core io_uring ...

Agreed, but this was needed that priv needs to have bvec than iovec.
Thinking about this, just adding bvec or make union with iov is
simple way to do this.

> 
> >   	struct iovec *iov;
> >   	struct iov_iter iter;
> >   	struct extent_state *cached_state;
> > @@ -4711,6 +4712,8 @@ static void btrfs_uring_read_finished(struct io_uring_cmd *cmd, unsigned int iss
> >   	kfree(priv->pages);
> >   	kfree(priv->iov);
> > +	if (priv->iou_vec.iovec)
> > +		kfree(priv->iou_vec.iovec);
> 
> ... exactly because if this. This line relies on details it
> shouldn't.

Yes, we don't need this.

Thanks,
Sidong

> 
> -- 
> Pavel Begunkov
>