Re: [PATCH 3/4] btrfs/ioctl: store btrfs_uring_encoded_data in io_btrfs_cmd

[Jens Axboe <axboe@kernel.dk>]

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