Dave Chinner wrote on Mon, May 01, 2023 at 09:32:41AM +1000: > > I've had a second look and I still don't see anything obvious though; > > I'd rather avoid adding a new variant of iterate()/iterate_shared() -- > > we could use that as a chance to add a flag to struct file_operation > > instead? e.g., something like mmap_supported_flags: > > I don't think that makes sense - the eventual goal is to make > ->iterate() go away entirely and all filesystems use > ->iterate_shared(). Hence I think adding flags to select iterate vs > iterate_shared and the locking that is needed is the wrong place to > start from here. (The flag could just go away when all filesystems not supporting it are gone, and it could be made the other way around (e.g. explicit NOT_SHARED to encourage migrations), so I don't really see the problem with this but next point makes this moot anyway) > Whether the filesystem supports non-blocking ->iterate_shared() or > not is a filesystem implementation option and io_uring needs that > information to be held on the struct file for efficient > determination of whether it should use non-blocking operations or > not. Right, sorry. I was thinking that since it's fs/op dependant it made more sense to keep next to the iterate operation, but that'd be a layering violation to look directly at the file_operation vector directly from the uring code... So having it in the struct file is better from that point of view. > We already set per-filesystem file modes via the ->open method, > that's how we already tell io_uring that it can do NOWAIT IO, as > well as async read/write IO for regular files. And now we also use > it for FMODE_DIO_PARALLEL_WRITE, too. > > See __io_file_supports_nowait().... > > Essentially, io_uring already cwhas the mechanism available to it > to determine if it should use NOWAIT semantics for getdents > operations; we just need to set FMODE_NOWAIT correctly for directory > files via ->open() on the filesystems that support it... Great, I wasn't aware of FMODE_NOWAIT; things are starting to fall in place. I'll send a v2 around Wed or Thurs (yay national holidays) > [ Hmmmm - we probably need to be more careful in XFS about what > types of files we set those flags on.... ] Yes, FMODE_NOWAIT will be set on directories as xfs_dir_open calls xfs_file_open which sets it inconditionally... So I got to check other filesystems don't do something similar as a bonus, but it looks like none that set FMODE_NOWAIT on regular files share the file open path, so at least that shouldn't be too bad. Happy to also fold the xfs fix as a prerequisite patch of this series or to let you do it, just tell me. Thanks, -- Dominique Martinet | Asmadeus
