From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Nick Piggin <npiggin@kernel.dk>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Al Viro <viro@ZenIV.linux.org.uk>,
linux-fsdevel@vger.kernel.org
Subject: Re: Odd NFS related SIGBUS (& possible fix)
Date: Fri, 01 Oct 2010 14:35:04 -0400 [thread overview]
Message-ID: <1285958104.30373.41.camel@heimdal.trondhjem.org> (raw)
In-Reply-To: <1285956768.30373.39.camel@heimdal.trondhjem.org>
On Fri, 2010-10-01 at 14:12 -0400, Trond Myklebust wrote:
> However, it looks to me as if the right thing to do when the
> page->mapping has changed would be to do the same thing as
> block_page_mkwrite(), and just return VM_FAULT_NOPAGE so that the VM can
> retry the fault.
> IMO: We should only SIGBUS if the calls to nfs_flush_incompatible()
> and/or nfs_updatepage() fail.
>
> Cheers
> Trond
IOW: Something like the following patch.
Cheers
Trond
---------------------------------------------------------------------------
NFS: Don't SIGBUS if nfs_vm_page_mkwrite races with a cache invalidation
From: Trond Myklebust <Trond.Myklebust@netapp.com>
In the case where we lock the page, and then find out that the page has
been thrown out of the page cache, we should just return VM_FAULT_NOPAGE.
This is what block_page_mkwrite() does in these situations.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
fs/nfs/file.c | 17 ++++++++---------
1 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 05bf3c0..6d95e24 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -551,7 +551,7 @@ static int nfs_vm_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
struct file *filp = vma->vm_file;
struct dentry *dentry = filp->f_path.dentry;
unsigned pagelen;
- int ret = -EINVAL;
+ int ret = VM_FAULT_NOPAGE;
struct address_space *mapping;
dfprintk(PAGECACHE, "NFS: vm_page_mkwrite(%s/%s(%ld), offset %lld)\n",
@@ -567,21 +567,20 @@ static int nfs_vm_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
if (mapping != dentry->d_inode->i_mapping)
goto out_unlock;
- ret = 0;
pagelen = nfs_page_length(page);
if (pagelen == 0)
goto out_unlock;
- ret = nfs_flush_incompatible(filp, page);
- if (ret != 0)
- goto out_unlock;
+ ret = VM_FAULT_LOCKED;
+ if (nfs_flush_incompatible(filp, page) == 0 &&
+ nfs_updatepage(filp, page, 0, pagelen) == 0)
+ goto out;
- ret = nfs_updatepage(filp, page, 0, pagelen);
+ ret = VM_FAULT_SIGBUS;
out_unlock:
- if (!ret)
- return VM_FAULT_LOCKED;
unlock_page(page);
- return VM_FAULT_SIGBUS;
+out:
+ return ret;
}
static const struct vm_operations_struct nfs_file_vm_ops = {
next prev parent reply other threads:[~2010-10-01 18:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-29 4:33 Benjamin Herrenschmidt
2010-09-29 7:44 ` Benjamin Herrenschmidt
2010-10-01 5:57 ` Benjamin Herrenschmidt
2010-10-01 6:01 ` Benjamin Herrenschmidt
2010-10-01 17:18 ` J. Bruce Fields
2010-10-01 18:12 ` Trond Myklebust
2010-10-01 18:35 ` Trond Myklebust [this message]
2010-10-01 20:57 ` Benjamin Herrenschmidt
2010-10-18 23:43 ` Benjamin Herrenschmidt
2010-10-01 20:53 ` Benjamin Herrenschmidt
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=1285958104.30373.41.camel@heimdal.trondhjem.org \
--to=trond.myklebust@netapp.com \
--cc=benh@kernel.crashing.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=npiggin@kernel.dk \
--cc=viro@ZenIV.linux.org.uk \
/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
all inboxes | Powered by JetHome®