Commit 874f9463 authored by Trond Myklebust's avatar Trond Myklebust

NFS: Fix a regression in the read() syscall

When invalidating the page cache for a regular file, we want to first
sync all dirty data to disk and then call invalidate_inode_pages2().
The latter relies on nfs_launder_page() and nfs_release_page() to deal
respectively with dirty pages, and unstable written pages.

When commit 95905446 ("NFS: avoid deadlocks with loop-back mounted
NFS filesystems.") changed the behaviour of nfs_release_page(), then it
made it possible for invalidate_inode_pages2() to fail with an EBUSY.
Unfortunately, that error is then propagated back to read().

Let's therefore work around the problem for now by protecting the call
to sync the data and invalidate_inode_pages2() so that they are atomic
w.r.t. the addition of new writes.
Later on, we can revisit whether or not we still need nfs_launder_page()
and nfs_release_page().
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@primarydata.com>
parent ec3ca4e5
...@@ -178,7 +178,7 @@ nfs_file_read(struct kiocb *iocb, struct iov_iter *to) ...@@ -178,7 +178,7 @@ nfs_file_read(struct kiocb *iocb, struct iov_iter *to)
iocb->ki_filp, iocb->ki_filp,
iov_iter_count(to), (unsigned long) iocb->ki_pos); iov_iter_count(to), (unsigned long) iocb->ki_pos);
result = nfs_revalidate_mapping(inode, iocb->ki_filp->f_mapping); result = nfs_revalidate_mapping_protected(inode, iocb->ki_filp->f_mapping);
if (!result) { if (!result) {
result = generic_file_read_iter(iocb, to); result = generic_file_read_iter(iocb, to);
if (result > 0) if (result > 0)
...@@ -199,7 +199,7 @@ nfs_file_splice_read(struct file *filp, loff_t *ppos, ...@@ -199,7 +199,7 @@ nfs_file_splice_read(struct file *filp, loff_t *ppos,
dprintk("NFS: splice_read(%pD2, %lu@%Lu)\n", dprintk("NFS: splice_read(%pD2, %lu@%Lu)\n",
filp, (unsigned long) count, (unsigned long long) *ppos); filp, (unsigned long) count, (unsigned long long) *ppos);
res = nfs_revalidate_mapping(inode, filp->f_mapping); res = nfs_revalidate_mapping_protected(inode, filp->f_mapping);
if (!res) { if (!res) {
res = generic_file_splice_read(filp, ppos, pipe, count, flags); res = generic_file_splice_read(filp, ppos, pipe, count, flags);
if (res > 0) if (res > 0)
......
...@@ -1067,11 +1067,14 @@ static bool nfs_mapping_need_revalidate_inode(struct inode *inode) ...@@ -1067,11 +1067,14 @@ static bool nfs_mapping_need_revalidate_inode(struct inode *inode)
} }
/** /**
* nfs_revalidate_mapping - Revalidate the pagecache * __nfs_revalidate_mapping - Revalidate the pagecache
* @inode - pointer to host inode * @inode - pointer to host inode
* @mapping - pointer to mapping * @mapping - pointer to mapping
* @may_lock - take inode->i_mutex?
*/ */
int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping) static int __nfs_revalidate_mapping(struct inode *inode,
struct address_space *mapping,
bool may_lock)
{ {
struct nfs_inode *nfsi = NFS_I(inode); struct nfs_inode *nfsi = NFS_I(inode);
unsigned long *bitlock = &nfsi->flags; unsigned long *bitlock = &nfsi->flags;
...@@ -1120,7 +1123,12 @@ int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping) ...@@ -1120,7 +1123,12 @@ int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping)
nfsi->cache_validity &= ~NFS_INO_INVALID_DATA; nfsi->cache_validity &= ~NFS_INO_INVALID_DATA;
spin_unlock(&inode->i_lock); spin_unlock(&inode->i_lock);
trace_nfs_invalidate_mapping_enter(inode); trace_nfs_invalidate_mapping_enter(inode);
ret = nfs_invalidate_mapping(inode, mapping); if (may_lock) {
mutex_lock(&inode->i_mutex);
ret = nfs_invalidate_mapping(inode, mapping);
mutex_unlock(&inode->i_mutex);
} else
ret = nfs_invalidate_mapping(inode, mapping);
trace_nfs_invalidate_mapping_exit(inode, ret); trace_nfs_invalidate_mapping_exit(inode, ret);
clear_bit_unlock(NFS_INO_INVALIDATING, bitlock); clear_bit_unlock(NFS_INO_INVALIDATING, bitlock);
...@@ -1130,6 +1138,29 @@ int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping) ...@@ -1130,6 +1138,29 @@ int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping)
return ret; return ret;
} }
/**
* nfs_revalidate_mapping - Revalidate the pagecache
* @inode - pointer to host inode
* @mapping - pointer to mapping
*/
int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping)
{
return __nfs_revalidate_mapping(inode, mapping, false);
}
/**
* nfs_revalidate_mapping_protected - Revalidate the pagecache
* @inode - pointer to host inode
* @mapping - pointer to mapping
*
* Differs from nfs_revalidate_mapping() in that it grabs the inode->i_mutex
* while invalidating the mapping.
*/
int nfs_revalidate_mapping_protected(struct inode *inode, struct address_space *mapping)
{
return __nfs_revalidate_mapping(inode, mapping, true);
}
static unsigned long nfs_wcc_update_inode(struct inode *inode, struct nfs_fattr *fattr) static unsigned long nfs_wcc_update_inode(struct inode *inode, struct nfs_fattr *fattr)
{ {
struct nfs_inode *nfsi = NFS_I(inode); struct nfs_inode *nfsi = NFS_I(inode);
......
...@@ -356,6 +356,7 @@ extern int nfs_revalidate_inode(struct nfs_server *server, struct inode *inode); ...@@ -356,6 +356,7 @@ extern int nfs_revalidate_inode(struct nfs_server *server, struct inode *inode);
extern int nfs_revalidate_inode_rcu(struct nfs_server *server, struct inode *inode); extern int nfs_revalidate_inode_rcu(struct nfs_server *server, struct inode *inode);
extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *); extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *);
extern int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping); extern int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping);
extern int nfs_revalidate_mapping_protected(struct inode *inode, struct address_space *mapping);
extern int nfs_setattr(struct dentry *, struct iattr *); extern int nfs_setattr(struct dentry *, struct iattr *);
extern void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr, struct nfs_fattr *); extern void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr, struct nfs_fattr *);
extern void nfs_setsecurity(struct inode *inode, struct nfs_fattr *fattr, extern void nfs_setsecurity(struct inode *inode, struct nfs_fattr *fattr,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment