+ writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix.patch added to -mm tree

+ writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix.patch added to -mm tree

[akpm]

The patch titled
     writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix
has been added to the -mm tree.  Its filename is
     writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

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

See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
out what to do about this

The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/

------------------------------------------------------
Subject: writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix
From: Andrew Morton <akpm@linux-foundation.org>

add comment

Cc: Christoph Hellwig <hch@lst.de>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Itaru Kitayama <kitayama@cl.bb4u.ne.jp>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Mel Gorman <mel@linux.vnet.ibm.com>
Cc: Minchan Kim <minchan.kim@gmail.com>
Cc: Nick Piggin <npiggin@kernel.dk>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/block_dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN fs/block_dev.c~writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix fs/block_dev.c
--- a/fs/block_dev.c~writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix
+++ a/fs/block_dev.c
@@ -57,7 +57,7 @@ static void bdev_inode_switch_bdi(struct
 {
 	struct backing_dev_info *old = inode->i_data.backing_dev_info;
 
-	if (dst == old)
+	if (dst == old)			/* deadlock avoidance */
 		return;
 	bdi_lock_two(&old->wb, &dst->wb);
 	spin_lock(&inode->i_lock);
_

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

origin.patch
linux-next.patch
next-remove-localversion.patch
i-need-old-gcc.patch
arch-alpha-kernel-systblss-remove-debug-check.patch
drivers-i2c-busses-i2c-designware-corec-needs-delayh.patch
arch-x86-include-asm-delayh-fix-udelay-and-ndelay-for-8-bit-args.patch
drivers-gpu-drm-radeon-atomc-fix-warning.patch
leds-route-kbd-leds-through-the-generic-leds-layer.patch
backlight-add-backlight-type-fix.patch
backlight-add-backlight-type-fix-fix.patch
drivers-video-backlight-adp5520_blc-check-strict_strtoul-return-value-fix.patch
drivers-message-fusion-mptsasc-fix-warning.patch
osst-wrong-index-used-in-inner-loop-checkpatch-fixes.patch
drivers-scsi-osstc-fix-warning.patch
drbd-fix-warning.patch
drivers-usb-misc-usbtestc-fix-warning.patch
mm.patch
mm-nommu-sort-mm-mmap-list-properly-fix.patch
mm-per-node-vmstat-show-proper-vmstats-fix.patch
mm-mem-hotplug-update-pcp-stat_threshold-when-memory-hotplug-occur-fix.patch
mm-mmu_gather-rework-fix.patch
mm-uninline-large-generic-tlbh-functions.patch
mm-thp-optimize-memcg-charge-in-khugepaged-fix.patch
mm-convert-mm-cpu_vm_cpumask-into-cpumask_var_t-fix.patch
mm-convert-mm-cpu_vm_cpumask-into-cpumask_var_t-checkpatch-fixes.patch
writeback-sync-expired-inodes-first-in-background-writeback-fix.patch
vmscan-change-shrink_slab-interfaces-by-passing-shrink_control-fix.patch
vmscan-change-shrink_slab-interfaces-by-passing-shrink_control-fix-2.patch
vmscan-change-shrinker-api-by-passing-shrink_control-struct-fix.patch
vmscan-change-shrinker-api-by-passing-shrink_control-struct-fix-2.patch
writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-fix.patch
frv-duplicate-output_buffer-of-e03-checkpatch-fixes.patch
hpet-factor-timer-allocate-from-open.patch
arch-alpha-include-asm-ioh-s-extern-inline-static-inline.patch
init-calibratec-fix-for-critical-bogomips-intermittent-calculation-failure-checkpatch-fixes.patch
init-calibratec-fix-for-critical-bogomips-intermittent-calculation-failure-fix.patch
printk-allocate-kernel-log-buffer-earlier-v2-checkpatch-fixes.patch
printk-allocate-kernel-log-buffer-earlier-v2-fix.patch
lru_cache-use-correct-type-in-sizeof-for-allocation-fix.patch
percpu_counter-change-return-value-and-add-comments-fix.patch
lib-hexdumpc-make-hex2bin-return-the-updated-src-address.patch
fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix.patch
fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix-fix.patch
fs-ncpfs-inodec-suppress-used-uninitialised-warning.patch
drivers-rtc-rtc-mrstc-use-release_mem_region-after-request_mem_region-fix.patch
rtc-driver-for-pt7c4338-chip-checkpatch-fixes.patch
rtc-driver-for-pt7c4338-chip-fix.patch
documentation-accounting-getdelaysc-handle-sendto-failures.patch
mm-move-enum-vm_event_item-into-a-standalone-header-file.patch
add-the-pagefault-count-into-memcg-stats-fix.patch
cpusets-randomize-node-rotor-used-in-cpuset_mem_spread_node.patch
cpusets-randomize-node-rotor-used-in-cpuset_mem_spread_node-cpusets-initialize-spread-rotor-lazily-fix.patch
fs-partitions-efic-corrupted-guid-partition-tables-can-cause-kernel-oops-fix.patch
scatterlist-new-helper-functions.patch
scatterlist-new-helper-functions-update-fix.patch
kexec-remove-kmsg_dump_kexec-fix.patch
journal_add_journal_head-debug.patch
mutex-subsystem-synchro-test-module-fix.patch
slab-leaks3-default-y.patch
put_bh-debug.patch
memblock-add-input-size-checking-to-memblock_find_region.patch
memblock-add-input-size-checking-to-memblock_find_region-fix.patch

Re: + writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-f ix.patch added to -mm tree

[Christoph Hellwig]

On Tue, May 03, 2011 at 01:57:44PM -0700, akpm@linux-foundation.org wrote:
>  	struct backing_dev_info *old = inode->i_data.backing_dev_info;
>  
> -	if (dst == old)
> +	if (dst == old)			/* deadlock avoidance */

That's not an overly useful comment.  It should be a proper block coment
documentation how that we could ever end up with the same bdi as
destination and source.

Which is something I wanted to ask Hugh anyway - do you have traces explaining
how this happens for you?

Re: + writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-f ix.patch added to -mm tree

[Christoph Hellwig]

On Tue, May 03, 2011 at 01:57:44PM -0700, akpm@linux-foundation.org wrote:
>  	struct backing_dev_info *old = inode->i_data.backing_dev_info;
>  
> -	if (dst == old)
> +	if (dst == old)			/* deadlock avoidance */

That's not an overly useful comment.  It should be a proper block coment
documentation how that we could ever end up with the same bdi as
destination and source.

Which is something I wanted to ask Hugh anyway - do you have traces explaining
how this happens for you?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: + writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-f ix.patch added to -mm tree

[Hugh Dickins]

On Wed, May 4, 2011 at 2:49 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Tue, May 03, 2011 at 01:57:44PM -0700, akpm@linux-foundation.org wrote:
>>       struct backing_dev_info *old = inode->i_data.backing_dev_info;
>>
>> -     if (dst == old)
>> +     if (dst == old)                 /* deadlock avoidance */
>
> That's not an overly useful comment.  It should be a proper block coment
> documentation how that we could ever end up with the same bdi as
> destination and source.

I didn't put in a comment myself because it seemed obvious that we'd
want to avoid calling something named bdi_lock_two(a, b) when a and b
are the same; and I was expecting it to be obvious to you that
actually we could get here with a and b the same.  But apparently not.
 default_backing_dev_info?

>
> Which is something I wanted to ask Hugh anyway - do you have traces explaining
> how this happens for you?

Let's take out the patch and jot down the dmesg, omitting ? stale
lines and function offsets from the backtrace.

Something that may prove relevant: this is openSUSE 11.4, which cannot
boot my kernels unless I have CONFIG_DEVTMPFS=y; and I've set
CONFIG_DEVTMPFS_MOUNT=y too.  No initramfs.

...
VFS: Mounted root (ext2 filesystem) readonly on device 8:1.
devtmpfs: mounted
Freeing unused kernel memory: 352k freed
udev[162]: starting version 166
BUG: spinlock recursion on CPU#0, blkid/299
lock: 78690c30, .magic: dead4ead, .owner: blkid/299, .owner_cpu: 0
Pid: 299, comm: blkid Not tainted 2.6.39-rc5-mm1 #4
Call Trace:
spin_bug
do_raw_spin_lock
_raw_spin_lock_nested
bdi_lock_two
bdev_inode_switch_bdi
__blkdev_get
blkdev_get
blkdev_open
__dentry_open
nameidata_to_filp
do_last
path_openat
do_filp_open
do_sys_open
sys_open
sysenter_do_call

And indeed that "lock: 78690c30" falls inside my
default_backing_dev_info at 78690ae4.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: + writeback-split-inode_wb_list_lock-into-bdi_writebacklist_lock-fix-f ix.patch added to -mm tree

[Hugh Dickins]

On Wed, May 4, 2011 at 2:49 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Tue, May 03, 2011 at 01:57:44PM -0700, akpm@linux-foundation.org wrote:
>>       struct backing_dev_info *old = inode->i_data.backing_dev_info;
>>
>> -     if (dst == old)
>> +     if (dst == old)                 /* deadlock avoidance */
>
> That's not an overly useful comment.  It should be a proper block coment
> documentation how that we could ever end up with the same bdi as
> destination and source.

I didn't put in a comment myself because it seemed obvious that we'd
want to avoid calling something named bdi_lock_two(a, b) when a and b
are the same; and I was expecting it to be obvious to you that
actually we could get here with a and b the same.  But apparently not.
 default_backing_dev_info?

>
> Which is something I wanted to ask Hugh anyway - do you have traces explaining
> how this happens for you?

Let's take out the patch and jot down the dmesg, omitting ? stale
lines and function offsets from the backtrace.

Something that may prove relevant: this is openSUSE 11.4, which cannot
boot my kernels unless I have CONFIG_DEVTMPFS=y; and I've set
CONFIG_DEVTMPFS_MOUNT=y too.  No initramfs.

...
VFS: Mounted root (ext2 filesystem) readonly on device 8:1.
devtmpfs: mounted
Freeing unused kernel memory: 352k freed
udev[162]: starting version 166
BUG: spinlock recursion on CPU#0, blkid/299
lock: 78690c30, .magic: dead4ead, .owner: blkid/299, .owner_cpu: 0
Pid: 299, comm: blkid Not tainted 2.6.39-rc5-mm1 #4
Call Trace:
spin_bug
do_raw_spin_lock
_raw_spin_lock_nested
bdi_lock_two
bdev_inode_switch_bdi
__blkdev_get
blkdev_get
blkdev_open
__dentry_open
nameidata_to_filp
do_last
path_openat
do_filp_open
do_sys_open
sys_open
sysenter_do_call

And indeed that "lock: 78690c30" falls inside my
default_backing_dev_info at 78690ae4.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>