Re: [RFC PATCH v5 5/5] btrfs: ioctl: introduce btrfs_uring_import_iovec()

[Jens Axboe <axboe@kernel.dk>]

On 3/21/25 4:28 AM, Pavel Begunkov wrote:
> On 3/20/25 16:19, Sidong Yang wrote:
>> On Thu, Mar 20, 2025 at 12:01:42PM +0000, Pavel Begunkov wrote:
>>> On 3/19/25 06:12, Sidong Yang wrote:
>>>> This patch introduces btrfs_uring_import_iovec(). In encoded read/write
>>>> with uring cmd, it uses import_iovec without supporting fixed buffer.
>>>> btrfs_using_import_iovec() could use fixed buffer if cmd flags has
>>>> IORING_URING_CMD_FIXED.
>>>
>>> Looks fine to me. The only comment, it appears btrfs silently ignored
>>> IORING_URING_CMD_FIXED before, so theoretically it changes the uapi.
>>> It should be fine, but maybe we should sneak in and backport a patch
>>> refusing the flag for older kernels?
>>
>> I think it's okay to leave the old version as it is. Making it to refuse
>> the flag could break user application.
> 
> Just as this patch breaks it. The cmd is new and quite specific, likely
> nobody would notice the change. As it currently stands, the fixed buffer
> version of the cmd is going to succeed in 99% of cases on older kernels
> because we're still passing an iovec in, but that's only until someone
> plays remapping games after a registration and gets bizarre results.
> 
> It's up to btrfs folks how they want to handle that, either try to fix
> it now, or have a chance someone will be surprised in the future. My
> recommendation would be the former one.

I'd strongly recommend that the btrfs side check for valid flags and
error it. It's a new enough addition that this should not be a concern,
and silently ignoring (currently) unsupported flags rather than erroring
them is a mistake.

Sidong, please do send a patch for that so it can go into 6.13 stable
and 6.14 to avoid any confusion in this area in the future.

-- 
Jens Axboe