On Fri, Mar 21, 2025 at 05:17:15AM -0600, Jens Axboe wrote: > 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. Agreed, It could be seen as a bug that the flag is dismissed silently. I'll write a patch for this. Thanks, Sidong > > -- > Jens Axboe
