+ git-nfs-vs-nfs-convert-to-new-aops.patch added to -mm tree

+ git-nfs-vs-nfs-convert-to-new-aops.patch added to -mm tree

[akpm]

The patch titled
     git-nfs vs nfs-convert-to-new-aops
has been added to the -mm tree.  Its filename is
     git-nfs-vs-nfs-convert-to-new-aops.patch

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this

------------------------------------------------------
Subject: git-nfs vs nfs-convert-to-new-aops
From: Andrew Morton <akpm@linux-foundation.org>

nfi if this is correct.  How am I supposed to know how to work out what to put
in `copied' in write_end?

Cc: Nick Piggin <nickpiggin@yahoo.com.au>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 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;
_

Patches currently in -mm which might be from akpm@linux-foundation.org are

origin.patch
sparsemem-ensure-we-initialise-the-node-mapping-for-sparsemem_static.patch
serial-8250-handle-saving-the-clear-on-read-bits-from-the-lsr.patch
add-blacklisting-capability-to-serial_pci-to-avoid-misdetection.patch
free_irq-fix-debug_shirq-handling.patch
remove-bdput-from-do_open-in-fs-block_devc-fix.patch
slow-down-printk-during-boot.patch
slow-down-printk-during-boot-fix-2.patch
git-acpi.patch
acpi-add-reboot-mechanism.patch
git-alsa.patch
working-3d-dri-intel-agpko-resume-for-i815-chip.patch
git-dma-up-fix.patch
git-dvb.patch
git-dvb-fixup-2.patch
adbhid-produce-all-capslock-key-events.patch
iforce-warning-fix.patch
git-kvm.patch
git-libata-all.patch
ata-add-the-sw-ncq-support-to-sata_nv-for-mcp51-mcp55-mcp61.patch
ide-arm-hack.patch
git-mmc-fixup.patch
git-mtd.patch
git-mtd-fix-printk-warning-in-jffs2_block_check_erase.patch
mtdoops-printk-warning-fixes.patch
git-netdev-all.patch
e1000e-build-fix.patch
revert-8139too-clean-up-i-o-remapping.patch
git-ixgbe-build-fix.patch
git-net.patch
git-net-fixup.patch
git-backlight.patch
git-nfsd.patch
revert-gregkh-pci-pci_bridge-device.patch
i386-add-support-for-picopower-irq-router.patch
pci-remove-irritating-try-pci=assign-busses-warning.patch
git-scsi-misc.patch
git-unionfs.patch
git-unionfs-build-fix.patch
git-unionfs-fix-2.patch
git-watchdog.patch
git-wireless.patch
git-wireless-fixup.patch
net-add-ath5k-wireless-driver-fix.patch
x86_64-dynticks-disable-hpet_id_legsup-hpets.patch
x86_64-get-mp_bus_to_node-as-early-v3.patch
git-xfs.patch
git-kgdb-fixup.patch
pci-align-bar-settings-for-legacy-mode-ide.patch
bridge-sysfs-fix.patch
vmscan-give-referenced-active-and-unmapped-pages-a-second-trip-around-the-lru.patch
sparsemem-record-when-a-section-has-a-valid-mem_map-fix.patch
readahead-combine-file_ra_stateprev_index-prev_offset-into-prev_pos-fix.patch
readahead-combine-file_ra_stateprev_index-prev_offset-into-prev_pos-fix-2.patch
vm-dont-run-touch_buffer-during-buffercache-lookups.patch
fs-introduce-write_begin-write_end-and-perform_write-aops.patch
git-nfs-vs-nfs-convert-to-new-aops.patch
memoryless-nodes-introduce-mask-of-nodes-with-memory-fix.patch
categorize-gfp-flags-fix.patch
bias-the-location-of-pages-freed-for-min_free_kbytes-in-the-same-max_order_nr_pages-blocks.patch
maps2-move-the-page-walker-code-to-lib.patch
maps2-add-proc-pid-pagemap-interface.patch
maps2-make-proc-pid-smaps-optional-under-config_embeddedpatch-fix.patch
slub-slab-validation-move-tracking-information-alloc-outside-of-melstuff.patch
hugetlbfs-read-support-fix.patch
security-convert-lsm-into-a-static-interface-fix.patch
security-convert-lsm-into-a-static-interface-fix-2.patch
security-convert-lsm-into-a-static-interface-fix-unionfs.patch
file-capabilities-clear-caps-cleanup-fix.patch
capabilityh-remove-include-of-currenth.patch
serial-turn-serial-console-suspend-a-boot-rather-than-compile-time-option-update.patch
cache-pipe-buf-page-address-for-non-highmem-arch.patch
softlockup-add-a-proc-tuning-parameter-fix.patch
force-erroneous-inclusions-of-compiler-h-files-to-be-errors-fix.patch
driver-for-the-atmel-on-chip-ssc-on-at32ap-and-at91-fix.patch
add-kernel-notifierc-fix.patch
do_sys_poll-simplify-playing-with-on-stack-data-fix.patch
pcmcia-compactflash-driver-for-pa-semi-electra-boards.patch
add-in-sunos-41x-compatible-mode-for-ufs-fix.patch
core_pattern-fix-up-a-few-miscellaneous-bugs-fix.patch
printk-add-interfaces-for-external-access-to-the-log-buffer-fix.patch
deprecate-aout-elf-interpreters-fix.patch
ext2-ext3-ext4-add-block-bitmap-validation-fix.patch
writeback-fix-time-ordering-of-the-per-superblock-dirty-inode-lists.patch
writeback-fix-time-ordering-of-the-per-superblock-dirty-inode-lists-2.patch
writeback-fix-time-ordering-of-the-per-superblock-dirty-inode-lists-3.patch
writeback-fix-time-ordering-of-the-per-superblock-dirty-inode-lists-4.patch
writeback-fix-comment-use-helper-function.patch
writeback-fix-time-ordering-of-the-per-superblock-dirty-inode-lists-5.patch
writeback-fix-time-ordering-of-the-per-superblock-dirty-inode-lists-6.patch
writeback-fix-time-ordering-of-the-per-superblock-dirty-inode-lists-7.patch
revert-faster-ext2_clear_inode.patch
ecryptfs-printk-warning-fixes.patch
tdfxfb-mtrr-support-fix.patch
fbdev-fb_create_modedb-non-static-int-first-=-1-fix.patch
intel-iommu-pci-generic-helper-function.patch
intel-iommu-iova-allocation-and-management-routines.patch
intel-iommu-intel-iommu-driver.patch
intel-iommu-iommu-floppy-workaround.patch
64-bit-i_version-afs-fixes.patch
revoke-wire-up-i386-system-calls.patch
revoke-vs-git-block.patch
task-containersv11-basic-task-container-framework-fix.patch
add-containerstats-v3-fix.patch
pid-namespaces-dynamic-kmem-cache-allocator-for-pid-namespaces-fix.patch
pid-namespaces-define-is_global_init-and-is_container_init-fix.patch
pid-namespaces-define-is_global_init-and-is_container_init-versus-x86_64-mm-i386-show-unhandled-signals-v3.patch
fs-superc-use-list_for_each_entry-instead-of-list_for_each-fix.patch
pid-namespaces-changes-to-show-virtual-ids-to-user-fix.patch
reiser4.patch
git-block-vs-reiser4.patch
page-owner-tracking-leak-detector.patch
check_dirty_inode_list.patch
w1-build-fix.patch

Re: + git-nfs-vs-nfs-convert-to-new-aops.patch added to -mm tree

[Peter Zijlstra]

On Mon, 20 Aug 2007 15:56:10 -0700 akpm@linux-foundation.org wrote:


> ------------------------------------------------------
> Subject: git-nfs vs nfs-convert-to-new-aops
> From: Andrew Morton <akpm@linux-foundation.org>
> 
> 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 <nickpiggin@yahoo.com.au>
> Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  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;
 }
 

Re: + git-nfs-vs-nfs-convert-to-new-aops.patch added to -mm tree

[Trond Myklebust]

On Thu, 2007-09-20 at 13:20 +0200, Peter Zijlstra wrote:

> > Cc: Nick Piggin <nickpiggin@yahoo.com.au>
> > Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > 
> >  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

Re: + git-nfs-vs-nfs-convert-to-new-aops.patch added to -mm tree

[Peter Zijlstra]

On Thu, 20 Sep 2007 13:20:47 +0200 Peter Zijlstra
<peterz@infradead.org> wrote:

> /me continues the mmap write on nfs adventure...

My test prog reliably hangs like so:

mm_tester     D 000000000040b305     0  2701   2699
6042cef0 602ca520 617dfa50 617de000 617dfa90 60010b62 617dfa80 6002785d
       617de000 60583840 6042bcc0 6042ca40 617dfad0 6019e3c2 00080000 617de000
       617dfae0 ffffc0e7 617dfb38 00000002 617dfb60 6019eb20 616e7ae0 6048bd00 Call Trace:
Call Trace:
617dfa58:  [<60010b62>] _switch_to+0x81/0xf5
617dfa68:  [<6002785d>] pick_next_entity+0x1a/0x38
617dfa98:  [<6019e3c2>] schedule+0x1b8/0x23f
617dfad8:  [<6019eb20>] schedule_timeout+0xa2/0xcb
617dfaf8:  [<60035dc3>] process_timeout+0x0/0xb
617dfb10:  [<6019eb1b>] schedule_timeout+0x9d/0xcb
617dfb68:  [<6019ea62>] io_schedule_timeout+0xf/0x17
617dfb78:  [<6004f0d1>] sync_page+0x6b/0x6f
617dfb88:  [<6019ecd3>] __wait_on_bit_lock+0x42/0x78
617dfbb0:  [<6004fa2a>] find_lock_page+0xb4/0x155
617dfbc8:  [<6004f806>] __lock_page+0x73/0xb3
617dfbf0:  [<60040585>] wake_bit_function+0x0/0x2a
617dfc38:  [<6004fa3c>] find_lock_page+0xc6/0x155
617dfc48:  [<600570c9>] do_page_cache_readahead+0x52/0x5f
617dfc78:  [<600508ea>] filemap_fault+0x151/0x2c2
617dfce8:  [<6005e9c8>] __do_fault+0x6c/0x444
617dfd68:  [<6005edd1>] do_linear_fault+0x31/0x33
617dfd88:  [<6005f04e>] handle_mm_fault+0x130/0x228
617dfda8:  [<6011e2d7>] __up_read+0x73/0x7b
617dfde8:  [<600131d4>] handle_page_fault+0x120/0x2d9
617dfe08:  [<601242c8>] tty_write+0x1f7/0x212
617dfe48:  [<60013513>] segv+0xac/0x286
617dff28:  [<60013461>] segv_handler+0x68/0x6e
617dff48:  [<600232c9>] get_skas_faultinfo+0x9c/0xa1
617dff68:  [<6002386f>] userspace+0x13a/0x19d
617dffc8:  [<60010d4c>] fork_handler+0x86/0x8d


A new nfs_sync_page() method tells me:

sleeping on page: 0000000060ba05c0 held by: [<000000006004f5d1>] add_to_page_cache_lru+0xf/0x3a


And a rather crude printk() and dump_stack() in add_to_page_cache_lru()
match:

page: 0000000060ba05c0
Call Trace:
605eda88:  [<6004f5f4>] add_to_page_cache_lru+0x32/0x3a
605edaa8:  [<60056ddc>] read_cache_pages+0x4a/0x8f
605edae8:  [<600f8e49>] nfs_readpages+0x116/0x164
605edb38:  [<600f86bb>] nfs_pagein_one+0x0/0xd2
605edb98:  [<60056e58>] read_pages+0x37/0x9b
605edbd8:  [<60056fbc>] __do_page_cache_readahead+0x100/0x146
605edc48:  [<600570dd>] do_page_cache_readahead+0x52/0x5f
605edc78:  [<600508f4>] filemap_fault+0x145/0x2c2
605edca8:  [<60022b7d>] run_syscall_stub+0xd1/0xdd
605edce8:  [<6005e9dc>] __do_fault+0x6c/0x444
605edd68:  [<6005ede5>] do_linear_fault+0x31/0x33
605edd88:  [<6005f062>] handle_mm_fault+0x130/0x228
605edda8:  [<6011e2eb>] __up_read+0x73/0x7b
605edde8:  [<600131d4>] handle_page_fault+0x120/0x2d9
605ede08:  [<601242dc>] tty_write+0x1f7/0x212
605ede48:  [<60013513>] segv+0xac/0x286
605edf28:  [<60013461>] segv_handler+0x68/0x6e
605edf48:  [<600232c9>] get_skas_faultinfo+0x9c/0xa1
605edf68:  [<6002386f>] userspace+0x13a/0x19d
605edfc8:  [<60010d4c>] fork_handler+0x86/0x8d


/me wonders, missing RPC request or locking mistake...