lkml.org 
[lkml]   [2010]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Odd NFS related SIGBUS (& possible fix)
From
Date
On Sat, 2010-10-02 at 06:57 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2010-10-01 at 14:35 -0400, Trond Myklebust wrote:
> > 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.
>
> Thanks, I'll try it but it will have to wait about 10 days as I'm off on
> vacation.
>
> I'll see if somebody here can do some tests while I'm away

BTW. I got some reports from testers that the patch fixes the issue.

Thanks !

Cheers,
Ben.

> Cheers,
> Ben.
>
> > 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 = {
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/




\
 
 \ /
  Last update: 2010-10-19 01:47    [W:0.069 / U:0.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site