On Sun, Sep 26, 2021 at 09:42:43AM +1000, Dave Chinner wrote: > Ok, so if the filesystem is doing block mapping in the IO path now, > why does the swap file still need to map the file into a private > block mapping now? i.e all the work that iomap_swapfile_activate() > does for filesystems like XFS and ext4 - it's this completely > redundant now that we are doing block mapping during swap file IO > via iomap_dio_rw()? Hi Dave, Thanks for bringing up all these points. I think they all deserve to go into the documentation as "things to consider" for people implementing ->swap_rw for their filesystem. Something I don't think David perhaps made sufficiently clear is that regular DIO from userspace gets handled by ->read_iter and ->write_iter. This ->swap_rw op is used exclusive for, as the name suggests, swap DIO. So filesystems don't have to handle swap DIO and regular DIO the same way, and can split the allocation work between ->swap_activate and the iomap callback as they see fit (as long as they can guarantee the lack of deadlocks under memory pressure). There are several advantages to using the DIO infrastructure for swap: - unify block & net swap paths - allow filesystems to _see_ swap IOs instead of being bypassed - get rid of the swap extent rbtree - allow writing compound pages to swap files instead of splitting them - allow ->readpage to be synchronous for better error reporting - remove page_file_mapping() and page_file_offset() I suspect there are several problems with this patchset, but I'm not likely to have a chance to read it closely for a few days. If you have time to give the XFS parts a good look, that would be fantastic.