On Sat, Apr 29, 2023 at 05:07:32PM +0900, Dominique Martinet wrote: > Dominique Martinet wrote on Fri, Apr 28, 2023 at 03:14:52PM +0900: > > > AFAICT, the io_uring code wouldn't need to do much more other than > > > punt to the work queue if it receives a -EAGAIN result. Otherwise > > > the what the filesystem returns doesn't need to change, and I don't > > > see that we need to change how the filldir callbacks work, either. > > > We just keep filling the user buffer until we either run out of > > > cached directory data or the user buffer is full. > > > > [...] I'd like to confirm what the uring > > side needs to do before proceeding -- looking at the read/write path > > there seems to be a polling mechanism in place to tell uring when to > > look again, and I haven't looked at this part of the code yet to see > > what happens if no such polling is in place (does uring just retry > > periodically?) > > Ok so this part can work out as you said, I hadn't understood what you > meant by "punt to the work queue" but that should work from my new > understanding of the ring; we can just return EAGAIN if the non-blocking > variant doesn't have immediate results and call the blocking variant > when we're called again without IO_URING_F_NONBLOCK in flags. > (So there's no need to try to add a form of polling, although that is > possible if we ever become able to do that; I'll just forget about this > and be happy this part is easy) > > > That just leaves deciding if a filesystem handles the blocking variant > or not; ideally if we can know early (prep time) we can even mark > REQ_F_FORCE_ASYNC in flags to skip the non-blocking call for filesystems > that don't handle that and we get the best of both worlds. > > 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. 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. 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... [ Hmmmm - we probably need to be more careful in XFS about what types of files we set those flags on.... ] Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
