MIME-Version: 1.0
References: <163969801519.20885.3977673503103544412.stgit@noble.brown> <163969850314.20885.13214679186436457787.stgit@noble.brown>
In-Reply-To: <163969850314.20885.13214679186436457787.stgit@noble.brown>
From:   Mark Hemment <markhemm@googlemail.com>
Date:   Mon, 20 Dec 2021 15:02:31 +0000
Message-ID: <CANe_+Ujg_NXFkPq1FOzM-+TZEQ8Rm9qc1Q=bwL_zt3eV65yY-g@mail.gmail.com>
Subject: Re: [PATCH 10/18] NFS: swap IO handling is slightly different for
 O_DIRECT IO
To:     NeilBrown <neilb@suse.de>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        Chuck Lever <chuck.lever@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Mel Gorman <mgorman@suse.de>,
        Christoph Hellwig <hch@infradead.org>,
        David Howells <dhowells@redhat.com>, linux-nfs@vger.kernel.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 16 Dec 2021 at 23:58, NeilBrown <neilb@suse.de> wrote:
>
> 1/ Taking the i_rwsem for swap IO triggers lockdep warnings regarding
>    possible deadlocks with "fs_reclaim".  These deadlocks could, I believe,
>    eventuate if a buffered read on the swapfile was attempted.
>
>    We don't need coherence with the page cache for a swap file, and
>    buffered writes are forbidden anyway.  There is no other need for
>    i_rwsem during direct IO.  So never take it for swap_rw()

Agreed.  I cannot see an issue with not taking the sem.

> 2/ generic_write_checks() explicitly forbids writes to swap, and
>    performs checks that are not needed for swap.  So bypass it
>    for swap_rw().
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
...
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> @@ -625,7 +625,7 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from)
>                 return result;
>
>         if (iocb->ki_flags & IOCB_DIRECT)
> -               return nfs_file_direct_write(iocb, from);
> +               return nfs_file_direct_write(iocb, from, false);
>
>         dprintk("NFS: write(%pD2, %zu@%Ld)\n",
>                 file, iov_iter_count(from), (long long) iocb->ki_pos);

This code at the top of nfs/file.c should be removed;
    /* Hack for future NFS swap support */
    #ifndef IS_SWAPFILE
    # define IS_SWAPFILE(inode)     (0)
    #endif
As IS_SWAPFILE() is used just below this diff (to prevent buffered
writes), better be using a non-hacked version.

Mark