On Mon, 20 Aug 2007 15:56:10 -0700 [EMAIL PROTECTED] wrote:
> ------------------------------------------------------
> Subject: git-nfs vs nfs-convert-to-new-aops
> From: Andrew Morton <[EMAIL PROTECTED]>
>
> nfi if this is correct. How am I supposed to know how to work out what to put
> in `copied' in write_end?
I can has broken NFS :-)
nfs_write_begin wants to lock the page itself, but we pass it a locked
page.
> 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;
}
-
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/
