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. as commit message?