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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxx
> 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-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
