Re: [PATCH] NFS: Don't use page_file_mapping after removing the page

["Benjamin Coddington" <bcodding@redhat.com>]

On 4 Feb 2019, at 14:20, Benjamin Coddington wrote:

> On 4 Feb 2019, at 14:07, Benjamin Coddington wrote:
>
>> On 4 Feb 2019, at 14:03, Benjamin Coddington wrote:
>>
>>> On 4 Feb 2019, at 13:32, Trond Myklebust wrote:
>>>
>>>> On Mon, 2019-02-04 at 13:13 -0500, Benjamin Coddington wrote:
>>>>> If nfs_page_async_flush() removes the page from the mapping, then we
>>>>> can't use page_file_mapping() on it as nfs_updatepate() is wont to do
>>>>> when receiving an error.  Instead, simplify nfs_zap_mapping() to take
>>>>> the inode.
>>>>>
>>>>
>>>> Won't this break NFS swap?
>>>
>>> I guess it may, but then I wonder: is that a thing that anyone does?  It
>>> sounds like a terrible idea..
>>>
>>> I'm curious enough though, and doing some research on it.
>>
>> There's enough to commits in the tree fixing problem or supporting swap
>> on NFS, so I suppose I've just missed all that.
>
> So for diskless systems, ah.  It makes sense.
>
> But does it make any sense to set NFS_INO_INVALID_DATA on a swap file?
> Does anyone share them?
>
> Dazed and confused, but trying to continue..

So if the page is backing a swap file, nfs_zap_mapping() will fail to set
NFS_INO_INVALID_DATA because i_mapping.nrpages will always be 0.

But it looks like NFS has already decided to not care if the mapping needs
revalidation for swap, since we assume /shared/ swap files are insane.

So without testing swap on NFS at all, it seems safe.

This is all new to me, though, so I need experienced revalidation of this
reasoning myself.

Perhaps it needs a comment to explain why it is safe to do..  any thoughts?

Ben