On Sat, 18 Dec 2021, Anna Schumaker wrote: > Hi Neil, > > On Thu, Dec 16, 2021 at 7:07 PM NeilBrown <neilb@xxxxxxx> wrote: > > > > swap-over-NFS currently has a variety of problems. > > > > swap writes call generic_write_checks(), which always fails on a swap > > file, so it completely fails. > > Even without this, various deadlocks are possible - largely due to > > improvements in NFS memory allocation (using NOFS instead of ATOMIC) > > which weren't tested against swap-out. > > > > NFS is the only filesystem that has supported fs-based swap IO, and it > > hasn't worked for several releases, so now is a convenient time to clean > > up the swap-via-filesystem interfaces - we cannot break anything ! > > > > So the first few patches here clean up and improve various parts of the > > swap-via-filesystem code. ->activate_swap() is given a cleaner > > interface, a new ->swap_rw is introduced instead of burdening > > ->direct_IO, etc. > > > > Current swap-to-filesystem code only ever submits single-page reads and > > writes. These patches change that to allow multi-page IO when adjacent > > requests are submitted. Writes are also changed to be async rather than > > sync. This substantially speeds up write throughput for swap-over-NFS. > > > > Some of the NFS patches can land independently of the MM patches. A few > > require the MM patches to land first. > > Thanks for fixing swap-over-NFS! Looks like it passes all the > swap-related xfstests except for generic/357 on NFS v4.2. This test > checks that we get -EINVAL on a reflinked swapfile, but I'm not sure > if there is a way to check for that on the client side but if you have > any ideas it would be nice to get that test passing while you're at > it! Thanks for testing!. I think that testing that swap fails on a reflinked file is bogus. This isn't an important part of the API, it is just an internal implementation detail. I certainly understand that it could be problematic implementing swap on a reflinked file within XFS and it is perfectly acceptable to fail such a request. But if one day someone decided to implement it - should that be seen as a regression? Certainly over NFS there is no reason at all not to swap to a file that happens to be reflinked on the server. I don't think it even makes sense to test if the file has holes as the current nfs_swap_activate() does. I don't exactly object to the test, but I think it is misguided and pointless. Thanks, NeilBrown
