Re: [PATCH 00/18 V2] Repair SWAP-over-NFS

["NeilBrown" <neilb@suse.de>]

On Sat, 18 Dec 2021, Anna Schumaker wrote:
> Hi Neil,
> 
> On Thu, Dec 16, 2021 at 7:07 PM NeilBrown <neilb@suse.de> 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