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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <163994803576.25899.6298619065481174544@noble.neil.brown.name>
Date:   Mon, 20 Dec 2021 08:07:15 +1100
From:   "NeilBrown" <neilb@...e.de>
To:     "Anna Schumaker" <anna.schumaker@...app.com>
Cc:     "Trond Myklebust" <trond.myklebust@...merspace.com>,
        "Chuck Lever" <chuck.lever@...cle.com>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        "Mel Gorman" <mgorman@...e.de>,
        "Christoph Hellwig" <hch@...radead.org>,
        "David Howells" <dhowells@...hat.com>,
        "Linux NFS Mailing List" <linux-nfs@...r.kernel.org>,
        linux-mm@...ck.org,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 00/18 V2] Repair SWAP-over-NFS

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