From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: Andi Kleen <andi@firstfloor.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: [RFC PATCH 2/2] NFS: Fix a potential deadlock in nfs_file_mmap()
Date: Fri, 08 Jan 2010 19:56:24 -0500 [thread overview]
Message-ID: <20100109005624.7473.15560.stgit@localhost.localdomain> (raw)
In-Reply-To: <20100109005624.7473.33215.stgit@localhost.localdomain>
We cannot call nfs_invalidate_mapping() inside file->f_ops->mmap(), since
this would cause us to grab the inode->i_mutex while already holding the
current->mm->mmap_sem (thus causing a potential ABBA deadlock with the file
write code, which can grab those locks in the opposite order).
We can fix this situation for the mmap() system call by using the new
mmap_pgoff() callback, which is called prior to taking the
current->mm->mmap_sem mutex.
We also add ensure that open() invalidates the mapping if the inode data is
stale so that other users of mmap() (mainly the exec and uselib system
calls) get up to date data too.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
fs/nfs/file.c | 28 ++++++++++++++++++++++------
fs/nfs/inode.c | 4 ++++
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 6b89132..723e254 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -42,6 +42,9 @@ static int nfs_file_open(struct inode *, struct file *);
static int nfs_file_release(struct inode *, struct file *);
static loff_t nfs_file_llseek(struct file *file, loff_t offset, int origin);
static int nfs_file_mmap(struct file *, struct vm_area_struct *);
+static unsigned long nfs_file_mmap_pgoff(struct file * file,
+ unsigned long addr, unsigned long len, unsigned long prot,
+ unsigned long flags, unsigned long pgoff);
static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
struct pipe_inode_info *pipe,
size_t count, unsigned int flags);
@@ -78,6 +81,7 @@ const struct file_operations nfs_file_operations = {
.splice_write = nfs_file_splice_write,
.check_flags = nfs_check_flags,
.setlease = nfs_setlease,
+ .mmap_pgoff = nfs_file_mmap_pgoff,
};
const struct inode_operations nfs_file_inode_operations = {
@@ -290,24 +294,36 @@ nfs_file_splice_read(struct file *filp, loff_t *ppos,
static int
nfs_file_mmap(struct file * file, struct vm_area_struct * vma)
{
- struct dentry *dentry = file->f_path.dentry;
- struct inode *inode = dentry->d_inode;
int status;
dprintk("NFS: mmap(%s/%s)\n",
- dentry->d_parent->d_name.name, dentry->d_name.name);
+ file->f_path.dentry->d_parent->d_name.name,
+ file->f_path.dentry->d_name.name);
/* Note: generic_file_mmap() returns ENOSYS on nommu systems
* so we call that before revalidating the mapping
*/
status = generic_file_mmap(file, vma);
- if (!status) {
+ if (!status)
vma->vm_ops = &nfs_file_vm_ops;
- status = nfs_revalidate_mapping(inode, file->f_mapping);
- }
return status;
}
+static unsigned long
+nfs_file_mmap_pgoff(struct file * file, unsigned long addr,
+ unsigned long len, unsigned long prot,
+ unsigned long flags, unsigned long pgoff)
+{
+ struct inode *inode = file->f_path.dentry->d_inode;
+ int status;
+
+ status = nfs_revalidate_mapping(inode, file->f_mapping);
+ if (status < 0)
+ return status;
+
+ return generic_file_mmap_pgoff(file, addr, len, prot, flags, pgoff);
+}
+
/*
* Flush any dirty pages for this process, and check for write errors.
* The return status from this call provides a reliable indication of
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index faa0918..90e1d67 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -56,6 +56,8 @@
static int enable_ino64 = NFS_64_BIT_INODE_NUMBERS_ENABLED;
static void nfs_invalidate_inode(struct inode *);
+static int nfs_invalidate_mapping(struct inode *inode,
+ struct address_space *mapping);
static int nfs_update_inode(struct inode *, struct nfs_fattr *);
static struct kmem_cache * nfs_inode_cachep;
@@ -694,6 +696,8 @@ int nfs_open(struct inode *inode, struct file *filp)
nfs_file_set_open_context(filp, ctx);
put_nfs_open_context(ctx);
nfs_fscache_set_inode_cookie(inode, filp);
+ if (NFS_I(inode)->cache_validity & NFS_INO_INVALID_DATA)
+ nfs_invalidate_mapping(inode, filp->f_mapping);
return 0;
}
next prev parent reply other threads:[~2010-01-09 1:07 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-07 20:29 [GIT PULL] Please pull NFS client bugfixes Trond Myklebust
2010-01-07 21:00 ` Andi Kleen
2010-01-07 21:23 ` Peter Staubach
2010-01-07 21:35 ` Andi Kleen
2010-01-07 21:53 ` Trond Myklebust
2010-01-07 23:51 ` Andi Kleen
2010-01-08 0:14 ` Trond Myklebust
2010-01-08 0:34 ` Linus Torvalds
2010-01-08 0:45 ` Andi Kleen
2010-01-08 1:03 ` Trond Myklebust
2010-01-08 1:03 ` Trond Myklebust
2010-01-08 1:12 ` Linus Torvalds
2010-01-08 1:22 ` Trond Myklebust
2010-01-08 1:26 ` Trond Myklebust
2010-01-09 0:56 ` [RFC PATCH 0/2] Fix up the NFS mmap code Trond Myklebust
2010-01-09 0:56 ` [RFC PATCH 1/2] VFS: Add a mmap_file() callback to struct file_operations Trond Myklebust
2010-01-09 0:56 ` Trond Myklebust [this message]
2010-01-09 1:54 ` [RFC PATCH 2/2] NFS: Fix a potential deadlock in nfs_file_mmap() Al Viro
2010-01-09 1:17 ` [RFC PATCH 0/2] Fix up the NFS mmap code Linus Torvalds
2010-01-09 1:38 ` Al Viro
2010-01-09 1:46 ` Al Viro
2010-01-09 1:57 ` Linus Torvalds
2010-01-09 2:11 ` Al Viro
2010-01-09 2:22 ` Linus Torvalds
2010-01-09 2:30 ` Al Viro
2010-01-09 2:40 ` Al Viro
2010-01-09 2:43 ` Al Viro
2010-01-10 2:00 ` Andi Kleen
2010-01-08 1:30 ` [GIT PULL] Please pull NFS client bugfixes Linus Torvalds
2010-01-08 1:35 ` Linus Torvalds
2010-01-08 2:00 ` Linus Torvalds
2010-01-14 13:18 ` Peter Zijlstra
2010-01-08 5:19 ` Andi Kleen
2010-01-08 1:22 ` Linus Torvalds
2010-01-08 0:43 ` Andi Kleen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100109005624.7473.15560.stgit@localhost.localdomain \
--to=trond.myklebust@netapp.com \
--cc=andi@firstfloor.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).