On 10/7/24 3:20 PM, Al Viro wrote:
> On Mon, Oct 07, 2024 at 12:20:20PM -0600, Jens Axboe wrote:
>> On 10/7/24 12:09 PM, Jens Axboe wrote:
>>>>>> Questions on the io_uring side:
>>>>>> * you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
>>>>>> Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
>>>>>> Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
>>>>>> Am I missing something subtle here?
>>>>>
>>>>> Right, it could be allowed for fgetxattr on the io_uring side. Anything
>>>>> that passes in a struct file would be fair game to enable it on.
>>>>> Anything that passes in a path (eg a non-fd value), it obviously
>>>>> wouldn't make sense anyway.
>>>>
>>>> OK, done and force-pushed into #work.xattr.
>>>
>>> I just checked, and while I think this is fine to do for the 'fd' taking
>>> {s,g}etxattr, I don't think the path taking ones should allow
>>> IOSQE_FIXED_FILE being set. It's nonsensical, as they don't take a file
>>> descriptor. So I'd prefer if we kept it to just the f* variants. I can
>>> just make this tweak in my io_uring 6.12 branch and get it upstream this
>>> week, that'll take it out of your hands.
>>>
>>> What do you think?
>>
>> Like the below. You can update yours if you want, or I can shove this
>> into 6.12, whatever is the easiest for you.
>
> Can I put your s-o-b on that, with e.g.
>
> io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE
>
> Rejection of IOSQE_FIXED_FILE combined with IORING_OP_[GS]ETXATTR
> is fine - these do not take a file descriptor, so such combination
> makes no sense. The checks are misplaced, though - as it is, they
> triggers on IORING_OP_F[GS]ETXATTR as well, and those do take
> a file reference, no matter the origin.
Yep that's perfect, officially:
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
Thanks Al!
--
Jens Axboe
