On Thu, Mar 23, 2023 at 12:30:30PM +0100, Jan Kara wrote: > > > > One way which hch is suggesting is to use __iomap_dio_rw() -> unlock > > inode -> call generic_write_sync(). I haven't yet worked on this part. > > So I see two problems with what Christoph suggests: > > a) It is unfortunate API design to require trivial (and low maintenance) > filesystem to do these relatively complex locking games. But this can > be solved by providing appropriate wrapper for them I guess. Well, the problem is that this "trivial" file systems have a pretty broken locking scheme for fsync. The legacy dio code gets around this by unlocking i_rwsem inside of __blockdev_direct_IO. Which force a particular and somewhat odd locking scheme on the callers, and severly limits it what it can do. > > Are you suggesting to rip of inode_lock from __generic_file_fsync()? > > Won't it have a much larger implications? > > Yes and yes :). But inode writeback already happens from other paths > without inode_lock so there's hardly any surprise there. > sync_mapping_buffers() is impossible to "customize" by filesystems and the > generic code is fine without inode_lock. So I have hard time imagining how > any filesystem would really depend on inode_lock in this path (famous last > words ;)). Not holding the inode lock in ->fsync would solve all of this indeed.
