Re: [PATCH] NFS: flush data when locking a file to ensure cache coherence for mmap.

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 2267 bytes --]


PING again

Thanks,
NeilBrown

On Fri, Jul 22 2016, NeilBrown wrote:

> PING .... any comments on this patch, or the problem it tries to
> address?
>
> Thanks,
> NeilBrown
>
>
> On Fri, Jun 03 2016, NeilBrown wrote:
>
>> When a byte range lock (or flock) is taken out on an NFS file, the
>> validity of the cached data is checked and the inode is marked
>> NFS_INODE_INVALID_DATA.  However the cached data isn't flushed from
>> the page cache.
>>
>> This is sufficient for future read() requests or mmap() requests as
>> they call nfs_revalidate_mapping() which performs the flush if
>> necessary.
>>
>> However an existing mapping is not affected.  Accessing data through
>> that mapping will continue to return old data even though the inode is
>> marked NFS_INODE_INVALID_DATA.
>>
>> This can easily be confirmed using the 'nfs' tool in
>>   git://github.com/okirch/twopence-nfs.git
>> and running
>>
>>    nfs coherence FILENAME
>> on one client, and
>>    nfs coherence -r FILENAME
>> on another client.
>>
>> It appears that prior to Linux 2.6.0 this worked correctly.
>>
>> However commit:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=ca9268fe3ddd075714005adecd4afbd7f9ab87d0
>>
>> removed the call to inode_invalidate_pages() from nfs_zap_caches().  I
>> haven't tested this code, but inspection suggests that prior to this
>> commit, file locking would invalidate all inode pages.
>>
>> This patch adds a call to nfs_revalidate_mapping_protected() after a
>> successful SETLK so that invalid data is flushed.  With this patch the
>> above test passes.
>>
>> Cc: Olaf Kirch <okir@suse.com>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>>  fs/nfs/file.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index 717a8d6af52d..781cc6c9c60b 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -822,6 +822,7 @@ do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
>>  			__nfs_revalidate_inode(NFS_SERVER(inode), inode);
>>  		else
>>  			nfs_zap_caches(inode);
>> +		nfs_revalidate_mapping_protected(inode, filp->f_mapping);
>>  	}
>>  out:
>>  	return status;
>> -- 
>> 2.8.3

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]