On Thu, 2007-09-20 at 13:20 +0200, Peter Zijlstra wrote:
> > Cc: Nick Piggin <[EMAIL PROTECTED]>
> > Cc: Trond Myklebust <[EMAIL PROTECTED]>
> > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
> > ---
> >
> > fs/nfs/file.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff -puN fs/nfs/file.c~git-nfs-vs-nfs-convert-to-new-aops fs/nfs/file.c
> > --- a/fs/nfs/file.c~git-nfs-vs-nfs-convert-to-new-aops
> > +++ a/fs/nfs/file.c
> > @@ -392,6 +392,7 @@ static int nfs_vm_page_mkwrite(struct vm
> > struct file *filp = vma->vm_file;
> > unsigned pagelen;
> > int ret = -EINVAL;
> > + void *fsdata;
> >
> > lock_page(page);
> > if (page->mapping != vma->vm_file->f_path.dentry->d_inode->i_mapping)
> > @@ -399,9 +400,13 @@ static int nfs_vm_page_mkwrite(struct vm
> > pagelen = nfs_page_length(page);
> > if (pagelen == 0)
> > goto out_unlock;
> > - ret = nfs_prepare_write(filp, page, 0, pagelen);
> > + ret = nfs_write_begin(filp, page->mapping,
> > + (loff_t)page->index << PAGE_CACHE_SHIFT,
> > + pagelen, 0, &page, &fsdata);
> > if (!ret)
> > - ret = nfs_commit_write(filp, page, 0, pagelen);
> > + ret = nfs_write_end(filp, page->mapping,
> > + (loff_t)page->index << PAGE_CACHE_SHIFT,
> > + pagelen, pagelen, page, fsdata);
> > out_unlock:
> > unlock_page(page);
> > return ret;
> > _
>
> But even with this patch I deadlock on page lock, just not here
> anymore :-/
>
> /me continues the mmap write on nfs adventure...
>
> ---
> fs/nfs/file.c | 36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> Index: linux-2.6/fs/nfs/file.c
> ===================================================================
> --- linux-2.6.orig/fs/nfs/file.c
> +++ linux-2.6/fs/nfs/file.c
> @@ -393,22 +393,34 @@ static int nfs_vm_page_mkwrite(struct vm
> unsigned pagelen;
> int ret = -EINVAL;
> void *fsdata;
> + struct address_space *mapping;
> + loff_t offset;
>
> lock_page(page);
> - if (page->mapping != vma->vm_file->f_path.dentry->d_inode->i_mapping)
> - goto out_unlock;
> + mapping = page->mapping;
> + if (mapping != vma->vm_file->f_path.dentry->d_inode->i_mapping) {
> + unlock_page(page);
> + return -EINVAL;
> + }
> pagelen = nfs_page_length(page);
> - if (pagelen == 0)
> - goto out_unlock;
> - ret = nfs_write_begin(filp, page->mapping,
> - (loff_t)page->index << PAGE_CACHE_SHIFT,
> - pagelen, 0, &page, &fsdata);
> - if (!ret)
> - ret = nfs_write_end(filp, page->mapping,
> - (loff_t)page->index << PAGE_CACHE_SHIFT,
> - pagelen, pagelen, page, fsdata);
> -out_unlock:
> + offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
> unlock_page(page);
> +
> + /*
> + * we can use mapping after releasing the page lock, because:
> + * we hold mmap_sem on the fault path, which should pin the vma
> + * which should pin the file, which pins the dentry which should
> + * hold a reference on inode.
> + */
> +
> + if (pagelen) {
> + struct page *page2 = NULL;
> + ret = nfs_write_begin(filp, mapping, offset, pagelen,
> + 0, &page2, &fsdata);
> + if (!ret)
> + ret = nfs_write_end(filp, mapping, offset, pagelen,
> + pagelen, page2, fsdata);
> + }
> return ret;
> }
BTW: Ideally, we want to replace this with a "generic_vm_page_mkwrite()"
in mm/filemap.c(?). There is nothing here which is NFS-specific (except
for the fact that we hard-code the callbacks).
Cheers
Trond
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
