Re: [tree] latest kill-the-BKL tree, v12

6 messages, 4 authors, 2009-04-16 · open the first message on its own page

Re: [tree] latest kill-the-BKL tree, v12

From: Ingo Molnar <hidden>
Date: 2009-04-15 23:08:02

* Alexander Beregalov [off-list ref] wrote:
2009/4/14 Ingo Molnar [off-list ref]:
quoted
* Alexander Beregalov [off-list ref] wrote:
quoted
On Tue, Apr 14, 2009 at 05:34:22AM +0200, Frederic Weisbecker wrote:
quoted
Ingo,

This small patchset fixes some deadlocks I've faced after trying
some pressures with dbench on a reiserfs partition.

There is still some work pending such as adding some checks to ensure we
_always_ release the lock before sleeping, as you suggested.
Also I have to fix a lockdep warning reported by Alessio Igor Bogani.
And also some optimizations....

Thanks,
Frederic.

Frederic Weisbecker (3):
  kill-the-BKL/reiserfs: provide a tool to lock only once the write lock
  kill-the-BKL/reiserfs: lock only once in reiserfs_truncate_file
  kill-the-BKL/reiserfs: only acquire the write lock once in
    reiserfs_dirty_inode

 fs/reiserfs/inode.c         |   10 +++++++---
 fs/reiserfs/lock.c          |   26 ++++++++++++++++++++++++++
 fs/reiserfs/super.c         |   15 +++++++++------
 include/linux/reiserfs_fs.h |    2 ++
 4 files changed, 44 insertions(+), 9 deletions(-)
Hi

The same test - dbench on reiserfs on loop on sparc64.

[ INFO: possible circular locking dependency detected ]
2.6.30-rc1-00457-gb21597d-dirty #2
I'm wondering ... your version hash suggests you used vanilla
upstream as a base for your test. There's a string of other fixes
from Frederic in tip:core/kill-the-BKL branch, have you picked them
all up when you did your testing?

The most coherent way to test this would be to pick up the latest
core/kill-the-BKL git tree from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/kill-the-BKL
I did not know about this branch, now I am testing it and there is 
no more problem with that testcase (dbench).

I will continue testing.
thanks for testing it! It seems reiserfs with Frederic's changes 
appears to be more stable now on your system.

I saw your NFS circular locking kill-the-BKL problem report on LKML 
- also attached below.

Hopefully someone on the Cc: list with NFS experience can point out 
the BKL assumption that is causing this.

	Ingo

----- Forwarded message from Alexander Beregalov [off-list ref] -----

Date: Wed, 15 Apr 2009 22:08:01 +0400
From: Alexander Beregalov <redacted>
To: linux-kernel <redacted>,
	Ingo Molnar [off-list ref], linux-nfs@vger.kernel.org
Subject: [core/kill-the-BKL] nfs3: possible circular locking dependency

Hi

I have pulled core/kill-the-BKL on top of 2.6.30-rc2.

device: '0:18': device_add

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.30-rc2-00057-g30aa902-dirty #5
-------------------------------------------------------
mount.nfs/1740 is trying to acquire lock:
 (kernel_mutex){+.+.+.}, at: [<00000000006f32dc>] lock_kernel+0x28/0x3c

but task is already holding lock:
 (&type->s_umount_key#24/1){+.+.+.}, at: [<00000000004b88a0>] sget+0x228/0x36c

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&type->s_umount_key#24/1){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<0000000000469f5c>] down_write_nested+0x38/0x50
       [<00000000004b88a0>] sget+0x228/0x36c
       [<00000000005688fc>] nfs_get_sb+0x80c/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40

-> #0 (kernel_mutex){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<00000000006f0ebc>] mutex_lock_nested+0x48/0x380
       [<00000000006f32dc>] lock_kernel+0x28/0x3c
       [<00000000006d20ec>] rpc_wait_bit_killable+0x64/0x8c
       [<00000000006f0620>] __wait_on_bit+0x64/0xc0
       [<00000000006f06e4>] out_of_line_wait_on_bit+0x68/0x7c
       [<00000000006d2938>] __rpc_execute+0x150/0x2b4
       [<00000000006d2ac0>] rpc_execute+0x24/0x34
       [<00000000006cc338>] rpc_run_task+0x64/0x74
       [<00000000006cc474>] rpc_call_sync+0x58/0x7c
       [<00000000005717b0>] nfs3_rpc_wrapper+0x24/0xa0
       [<0000000000572024>] do_proc_get_root+0x6c/0x10c
       [<00000000005720dc>] nfs3_proc_get_root+0x18/0x5c
       [<000000000056401c>] nfs_get_root+0x34/0x17c
       [<0000000000568adc>] nfs_get_sb+0x9ec/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40

other info that might help us debug this:

1 lock held by mount.nfs/1740:
 #0:  (&type->s_umount_key#24/1){+.+.+.}, at: [<00000000004b88a0>]
sget+0x228/0x36c

stack backtrace:
Call Trace:
 [00000000004755ac] print_circular_bug_tail+0xfc/0x10c
 [0000000000476e24] __lock_acquire+0x12f0/0x1b40
 [00000000004776d0] lock_acquire+0x5c/0x74
 [00000000006f0ebc] mutex_lock_nested+0x48/0x380
 [00000000006f32dc] lock_kernel+0x28/0x3c
 [00000000006d20ec] rpc_wait_bit_killable+0x64/0x8c
 [00000000006f0620] __wait_on_bit+0x64/0xc0
 [00000000006f06e4] out_of_line_wait_on_bit+0x68/0x7c
 [00000000006d2938] __rpc_execute+0x150/0x2b4
 [00000000006d2ac0] rpc_execute+0x24/0x34
 [00000000006cc338] rpc_run_task+0x64/0x74
 [00000000006cc474] rpc_call_sync+0x58/0x7c
 [00000000005717b0] nfs3_rpc_wrapper+0x24/0xa0
 [0000000000572024] do_proc_get_root+0x6c/0x10c
 [00000000005720dc] nfs3_proc_get_root+0x18/0x5c
 [000000000056401c] nfs_get_root+0x34/0x17c
device: '0:19': device_add

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

Re: [tree] latest kill-the-BKL tree, v12

From: Trond Myklebust <hidden>
Date: 2009-04-15 23:14:16

On Thu, 2009-04-16 at 01:07 +0200, Ingo Molnar wrote:
* Alexander Beregalov [off-list ref] wrote:
quoted
2009/4/14 Ingo Molnar [off-list ref]:
quoted
* Alexander Beregalov [off-list ref] wrote:
quoted
On Tue, Apr 14, 2009 at 05:34:22AM +0200, Frederic Weisbecker wrote:
quoted
Ingo,

This small patchset fixes some deadlocks I've faced after trying
some pressures with dbench on a reiserfs partition.

There is still some work pending such as adding some checks to ensure we
_always_ release the lock before sleeping, as you suggested.
Also I have to fix a lockdep warning reported by Alessio Igor Bogani.
And also some optimizations....

Thanks,
Frederic.

Frederic Weisbecker (3):
  kill-the-BKL/reiserfs: provide a tool to lock only once the write lock
  kill-the-BKL/reiserfs: lock only once in reiserfs_truncate_file
  kill-the-BKL/reiserfs: only acquire the write lock once in
    reiserfs_dirty_inode

 fs/reiserfs/inode.c         |   10 +++++++---
 fs/reiserfs/lock.c          |   26 ++++++++++++++++++++++++++
 fs/reiserfs/super.c         |   15 +++++++++------
 include/linux/reiserfs_fs.h |    2 ++
 4 files changed, 44 insertions(+), 9 deletions(-)
Hi

The same test - dbench on reiserfs on loop on sparc64.

[ INFO: possible circular locking dependency detected ]
2.6.30-rc1-00457-gb21597d-dirty #2
I'm wondering ... your version hash suggests you used vanilla
upstream as a base for your test. There's a string of other fixes
from Frederic in tip:core/kill-the-BKL branch, have you picked them
all up when you did your testing?

The most coherent way to test this would be to pick up the latest
core/kill-the-BKL git tree from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/kill-the-BKL
I did not know about this branch, now I am testing it and there is 
no more problem with that testcase (dbench).

I will continue testing.
thanks for testing it! It seems reiserfs with Frederic's changes 
appears to be more stable now on your system.

I saw your NFS circular locking kill-the-BKL problem report on LKML 
- also attached below.

Hopefully someone on the Cc: list with NFS experience can point out 
the BKL assumption that is causing this.
I have no idea what Alexander is seeing. There should be no BKL
dependencies at all left in the RPC client code. Most of the NFS client
code is clean too, with only the posix lock code, and the NFSv4 callback
server remaining...

Cheers
  Trond

Re: [tree] latest kill-the-BKL tree, v12

From: Frederic Weisbecker <hidden>
Date: 2009-04-15 23:35:58

On Thu, Apr 16, 2009 at 01:07:36AM +0200, Ingo Molnar wrote:
* Alexander Beregalov [off-list ref] wrote:
quoted
2009/4/14 Ingo Molnar [off-list ref]:
quoted
* Alexander Beregalov [off-list ref] wrote:
quoted
On Tue, Apr 14, 2009 at 05:34:22AM +0200, Frederic Weisbecker wrote:
quoted
Ingo,

This small patchset fixes some deadlocks I've faced after trying
some pressures with dbench on a reiserfs partition.

There is still some work pending such as adding some checks to ensure we
_always_ release the lock before sleeping, as you suggested.
Also I have to fix a lockdep warning reported by Alessio Igor Bogani.
And also some optimizations....

Thanks,
Frederic.

Frederic Weisbecker (3):
  kill-the-BKL/reiserfs: provide a tool to lock only once the write lock
  kill-the-BKL/reiserfs: lock only once in reiserfs_truncate_file
  kill-the-BKL/reiserfs: only acquire the write lock once in
    reiserfs_dirty_inode

 fs/reiserfs/inode.c         |   10 +++++++---
 fs/reiserfs/lock.c          |   26 ++++++++++++++++++++++++++
 fs/reiserfs/super.c         |   15 +++++++++------
 include/linux/reiserfs_fs.h |    2 ++
 4 files changed, 44 insertions(+), 9 deletions(-)
Hi

The same test - dbench on reiserfs on loop on sparc64.

[ INFO: possible circular locking dependency detected ]
2.6.30-rc1-00457-gb21597d-dirty #2
I'm wondering ... your version hash suggests you used vanilla
upstream as a base for your test. There's a string of other fixes
from Frederic in tip:core/kill-the-BKL branch, have you picked them
all up when you did your testing?

The most coherent way to test this would be to pick up the latest
core/kill-the-BKL git tree from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/kill-the-BKL
I did not know about this branch, now I am testing it and there is 
no more problem with that testcase (dbench).

I will continue testing.
thanks for testing it! It seems reiserfs with Frederic's changes 
appears to be more stable now on your system.



Yeah, thanks a lot for this testing!


 
I saw your NFS circular locking kill-the-BKL problem report on LKML 
- also attached below.

Hopefully someone on the Cc: list with NFS experience can point out 
the BKL assumption that is causing this.

	Ingo

----- Forwarded message from Alexander Beregalov [off-list ref] -----

Date: Wed, 15 Apr 2009 22:08:01 +0400
From: Alexander Beregalov <redacted>
To: linux-kernel <redacted>,
	Ingo Molnar [off-list ref], linux-nfs@vger.kernel.org
Subject: [core/kill-the-BKL] nfs3: possible circular locking dependency

Hi

I have pulled core/kill-the-BKL on top of 2.6.30-rc2.

device: '0:18': device_add

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.30-rc2-00057-g30aa902-dirty #5
-------------------------------------------------------
mount.nfs/1740 is trying to acquire lock:
 (kernel_mutex){+.+.+.}, at: [<00000000006f32dc>] lock_kernel+0x28/0x3c

but task is already holding lock:
 (&type->s_umount_key#24/1){+.+.+.}, at: [<00000000004b88a0>] sget+0x228/0x36c

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&type->s_umount_key#24/1){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<0000000000469f5c>] down_write_nested+0x38/0x50
       [<00000000004b88a0>] sget+0x228/0x36c
       [<00000000005688fc>] nfs_get_sb+0x80c/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40

-> #0 (kernel_mutex){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<00000000006f0ebc>] mutex_lock_nested+0x48/0x380
       [<00000000006f32dc>] lock_kernel+0x28/0x3c
       [<00000000006d20ec>] rpc_wait_bit_killable+0x64/0x8c
       [<00000000006f0620>] __wait_on_bit+0x64/0xc0
       [<00000000006f06e4>] out_of_line_wait_on_bit+0x68/0x7c
       [<00000000006d2938>] __rpc_execute+0x150/0x2b4
       [<00000000006d2ac0>] rpc_execute+0x24/0x34
       [<00000000006cc338>] rpc_run_task+0x64/0x74
       [<00000000006cc474>] rpc_call_sync+0x58/0x7c
       [<00000000005717b0>] nfs3_rpc_wrapper+0x24/0xa0
       [<0000000000572024>] do_proc_get_root+0x6c/0x10c
       [<00000000005720dc>] nfs3_proc_get_root+0x18/0x5c
       [<000000000056401c>] nfs_get_root+0x34/0x17c
       [<0000000000568adc>] nfs_get_sb+0x9ec/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40



This is still the dependency between bkl and s_umount_key
that has been reported recently. I wonder if this is not a
problem in the fs layer. I should investigate on it.

Thanks.



 
other info that might help us debug this:

1 lock held by mount.nfs/1740:
 #0:  (&type->s_umount_key#24/1){+.+.+.}, at: [<00000000004b88a0>]
sget+0x228/0x36c

stack backtrace:
Call Trace:
 [00000000004755ac] print_circular_bug_tail+0xfc/0x10c
 [0000000000476e24] __lock_acquire+0x12f0/0x1b40
 [00000000004776d0] lock_acquire+0x5c/0x74
 [00000000006f0ebc] mutex_lock_nested+0x48/0x380
 [00000000006f32dc] lock_kernel+0x28/0x3c
 [00000000006d20ec] rpc_wait_bit_killable+0x64/0x8c
 [00000000006f0620] __wait_on_bit+0x64/0xc0
 [00000000006f06e4] out_of_line_wait_on_bit+0x68/0x7c
 [00000000006d2938] __rpc_execute+0x150/0x2b4
 [00000000006d2ac0] rpc_execute+0x24/0x34
 [00000000006cc338] rpc_run_task+0x64/0x74
 [00000000006cc474] rpc_call_sync+0x58/0x7c
 [00000000005717b0] nfs3_rpc_wrapper+0x24/0xa0
 [0000000000572024] do_proc_get_root+0x6c/0x10c
 [00000000005720dc] nfs3_proc_get_root+0x18/0x5c
 [000000000056401c] nfs_get_root+0x34/0x17c
device: '0:19': device_add

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

Re: [tree] latest kill-the-BKL tree, v12

From: Ingo Molnar <hidden>
Date: 2009-04-16 08:52:31

* Frederic Weisbecker [off-list ref] wrote:
On Thu, Apr 16, 2009 at 01:07:36AM +0200, Ingo Molnar wrote:
quoted
* Alexander Beregalov [off-list ref] wrote:
quoted
2009/4/14 Ingo Molnar [off-list ref]:
quoted
* Alexander Beregalov [off-list ref] wrote:
quoted
On Tue, Apr 14, 2009 at 05:34:22AM +0200, Frederic Weisbecker wrote:
quoted
Ingo,

This small patchset fixes some deadlocks I've faced after trying
some pressures with dbench on a reiserfs partition.

There is still some work pending such as adding some checks to ensure we
_always_ release the lock before sleeping, as you suggested.
Also I have to fix a lockdep warning reported by Alessio Igor Bogani.
And also some optimizations....

Thanks,
Frederic.

Frederic Weisbecker (3):
  kill-the-BKL/reiserfs: provide a tool to lock only once the write lock
  kill-the-BKL/reiserfs: lock only once in reiserfs_truncate_file
  kill-the-BKL/reiserfs: only acquire the write lock once in
    reiserfs_dirty_inode

 fs/reiserfs/inode.c         |   10 +++++++---
 fs/reiserfs/lock.c          |   26 ++++++++++++++++++++++++++
 fs/reiserfs/super.c         |   15 +++++++++------
 include/linux/reiserfs_fs.h |    2 ++
 4 files changed, 44 insertions(+), 9 deletions(-)
Hi

The same test - dbench on reiserfs on loop on sparc64.

[ INFO: possible circular locking dependency detected ]
2.6.30-rc1-00457-gb21597d-dirty #2
I'm wondering ... your version hash suggests you used vanilla
upstream as a base for your test. There's a string of other fixes
from Frederic in tip:core/kill-the-BKL branch, have you picked them
all up when you did your testing?

The most coherent way to test this would be to pick up the latest
core/kill-the-BKL git tree from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/kill-the-BKL
I did not know about this branch, now I am testing it and there is 
no more problem with that testcase (dbench).

I will continue testing.
thanks for testing it! It seems reiserfs with Frederic's changes 
appears to be more stable now on your system.



Yeah, thanks a lot for this testing!


 
quoted
I saw your NFS circular locking kill-the-BKL problem report on LKML 
- also attached below.

Hopefully someone on the Cc: list with NFS experience can point out 
the BKL assumption that is causing this.

	Ingo

----- Forwarded message from Alexander Beregalov [off-list ref] -----

Date: Wed, 15 Apr 2009 22:08:01 +0400
From: Alexander Beregalov <redacted>
To: linux-kernel <redacted>,
	Ingo Molnar [off-list ref], linux-nfs@vger.kernel.org
Subject: [core/kill-the-BKL] nfs3: possible circular locking dependency

Hi

I have pulled core/kill-the-BKL on top of 2.6.30-rc2.

device: '0:18': device_add

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.30-rc2-00057-g30aa902-dirty #5
-------------------------------------------------------
mount.nfs/1740 is trying to acquire lock:
 (kernel_mutex){+.+.+.}, at: [<00000000006f32dc>] lock_kernel+0x28/0x3c

but task is already holding lock:
 (&type->s_umount_key#24/1){+.+.+.}, at: [<00000000004b88a0>] sget+0x228/0x36c

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&type->s_umount_key#24/1){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<0000000000469f5c>] down_write_nested+0x38/0x50
       [<00000000004b88a0>] sget+0x228/0x36c
       [<00000000005688fc>] nfs_get_sb+0x80c/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40

-> #0 (kernel_mutex){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<00000000006f0ebc>] mutex_lock_nested+0x48/0x380
       [<00000000006f32dc>] lock_kernel+0x28/0x3c
       [<00000000006d20ec>] rpc_wait_bit_killable+0x64/0x8c
       [<00000000006f0620>] __wait_on_bit+0x64/0xc0
       [<00000000006f06e4>] out_of_line_wait_on_bit+0x68/0x7c
       [<00000000006d2938>] __rpc_execute+0x150/0x2b4
       [<00000000006d2ac0>] rpc_execute+0x24/0x34
       [<00000000006cc338>] rpc_run_task+0x64/0x74
       [<00000000006cc474>] rpc_call_sync+0x58/0x7c
       [<00000000005717b0>] nfs3_rpc_wrapper+0x24/0xa0
       [<0000000000572024>] do_proc_get_root+0x6c/0x10c
       [<00000000005720dc>] nfs3_proc_get_root+0x18/0x5c
       [<000000000056401c>] nfs_get_root+0x34/0x17c
       [<0000000000568adc>] nfs_get_sb+0x9ec/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40



This is still the dependency between bkl and s_umount_key that has 
been reported recently. I wonder if this is not a problem in the 
fs layer. I should investigate on it.
The problem seem to be that this NFS call context:

-> #0 (kernel_mutex){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<00000000006f0ebc>] mutex_lock_nested+0x48/0x380
       [<00000000006f32dc>] lock_kernel+0x28/0x3c
       [<00000000006d20ec>] rpc_wait_bit_killable+0x64/0x8c
       [<00000000006f0620>] __wait_on_bit+0x64/0xc0
       [<00000000006f06e4>] out_of_line_wait_on_bit+0x68/0x7c
       [<00000000006d2938>] __rpc_execute+0x150/0x2b4
       [<00000000006d2ac0>] rpc_execute+0x24/0x34
       [<00000000006cc338>] rpc_run_task+0x64/0x74
       [<00000000006cc474>] rpc_call_sync+0x58/0x7c
       [<00000000005717b0>] nfs3_rpc_wrapper+0x24/0xa0
       [<0000000000572024>] do_proc_get_root+0x6c/0x10c
       [<00000000005720dc>] nfs3_proc_get_root+0x18/0x5c
       [<000000000056401c>] nfs_get_root+0x34/0x17c
       [<0000000000568adc>] nfs_get_sb+0x9ec/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40

Can be called with the BKL held - and then it schedule()s with the 
BKL held, creating dependencies. I did the quick hack below (a year 
ago! :-) but indeed that's probably wrong: we just drop and then 
re-acquire the BKL at a very low level - inverting the dependency 
chain.

It's not a problem of the NFS code, it's the probem of 
vfs_kern_mount taking the BKL.

Maybe it would be better if nfs_get_sb() dropped the BKL (knowing 
that it's called with the BKL held) - since it does not rely on the 
BKL? Not rpc_wait_bit_killable().

	Ingo

-------------->
From 352e0d25def53e6b36234e4dc2083ca7f5d712a9 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <redacted>
Date: Wed, 14 May 2008 17:31:41 +0200
Subject: [PATCH] remove the BKL: restructure NFS code

the naked schedule() in rpc_wait_bit_killable() caused the BKL to
be auto-dropped in the past.

avoid the immediate hang in such code. Note that this still leaves
some other locking dependencies to be sorted out in the NFS code.

Signed-off-by: Ingo Molnar <redacted>
---
 net/sunrpc/sched.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 6eab9bf..e12e571 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -224,9 +224,15 @@ EXPORT_SYMBOL_GPL(rpc_destroy_wait_queue);
 
 static int rpc_wait_bit_killable(void *word)
 {
+	int bkl = kernel_locked();
+
 	if (fatal_signal_pending(current))
 		return -ERESTARTSYS;
+	if (bkl)
+		unlock_kernel();
 	schedule();
+	if (bkl)
+		lock_kernel();
 	return 0;
 }
 

Re: [tree] latest kill-the-BKL tree, v12

From: Alessio Igor Bogani <hidden>
Date: 2009-04-16 14:36:23

Dear Sir Molnar,

2009/4/16 Ingo Molnar [off-list ref]:
[...]
quoted
This is still the dependency between bkl and s_umount_key that has
been reported recently. I wonder if this is not a problem in the
fs layer. I should investigate on it.
The problem seem to be that this NFS call context:

-> #0 (kernel_mutex){+.+.+.}:
      [<00000000004776d0>] lock_acquire+0x5c/0x74
      [<00000000006f0ebc>] mutex_lock_nested+0x48/0x380
      [<00000000006f32dc>] lock_kernel+0x28/0x3c
      [<00000000006d20ec>] rpc_wait_bit_killable+0x64/0x8c
      [<00000000006f0620>] __wait_on_bit+0x64/0xc0
      [<00000000006f06e4>] out_of_line_wait_on_bit+0x68/0x7c
      [<00000000006d2938>] __rpc_execute+0x150/0x2b4
      [<00000000006d2ac0>] rpc_execute+0x24/0x34
      [<00000000006cc338>] rpc_run_task+0x64/0x74
      [<00000000006cc474>] rpc_call_sync+0x58/0x7c
      [<00000000005717b0>] nfs3_rpc_wrapper+0x24/0xa0
      [<0000000000572024>] do_proc_get_root+0x6c/0x10c
      [<00000000005720dc>] nfs3_proc_get_root+0x18/0x5c
      [<000000000056401c>] nfs_get_root+0x34/0x17c
      [<0000000000568adc>] nfs_get_sb+0x9ec/0xa7c
      [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
      [<00000000004b7f84>] do_kern_mount+0x30/0xcc
      [<00000000004cf300>] do_mount+0x7c8/0x80c
      [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
      [<0000000000406154>] linux_sparc_syscall32+0x34/0x40
Proposed patch that i just sent
(http://marc.info/?l=linux-kernel&m=123989213917572&w=2) seems fix the
lock dependency.
I don't know if it is the right way to solve the problem in any case
but it works on my laptop, at least.

Ciao,
Alessio
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [tree] latest kill-the-BKL tree, v12

From: Frederic Weisbecker <hidden>
Date: 2009-04-16 16:40:59

On Thu, Apr 16, 2009 at 10:51:53AM +0200, Ingo Molnar wrote:
* Frederic Weisbecker [off-list ref] wrote:
quoted
On Thu, Apr 16, 2009 at 01:07:36AM +0200, Ingo Molnar wrote:
quoted
* Alexander Beregalov [off-list ref] wrote:
quoted
2009/4/14 Ingo Molnar [off-list ref]:
quoted
* Alexander Beregalov [off-list ref] wrote:
quoted
On Tue, Apr 14, 2009 at 05:34:22AM +0200, Frederic Weisbecker wrote:
quoted
Ingo,

This small patchset fixes some deadlocks I've faced after trying
some pressures with dbench on a reiserfs partition.

There is still some work pending such as adding some checks to ensure we
_always_ release the lock before sleeping, as you suggested.
Also I have to fix a lockdep warning reported by Alessio Igor Bogani.
And also some optimizations....

Thanks,
Frederic.

Frederic Weisbecker (3):
  kill-the-BKL/reiserfs: provide a tool to lock only once the write lock
  kill-the-BKL/reiserfs: lock only once in reiserfs_truncate_file
  kill-the-BKL/reiserfs: only acquire the write lock once in
    reiserfs_dirty_inode

 fs/reiserfs/inode.c         |   10 +++++++---
 fs/reiserfs/lock.c          |   26 ++++++++++++++++++++++++++
 fs/reiserfs/super.c         |   15 +++++++++------
 include/linux/reiserfs_fs.h |    2 ++
 4 files changed, 44 insertions(+), 9 deletions(-)
Hi

The same test - dbench on reiserfs on loop on sparc64.

[ INFO: possible circular locking dependency detected ]
2.6.30-rc1-00457-gb21597d-dirty #2
I'm wondering ... your version hash suggests you used vanilla
upstream as a base for your test. There's a string of other fixes
from Frederic in tip:core/kill-the-BKL branch, have you picked them
all up when you did your testing?

The most coherent way to test this would be to pick up the latest
core/kill-the-BKL git tree from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/kill-the-BKL
I did not know about this branch, now I am testing it and there is 
no more problem with that testcase (dbench).

I will continue testing.
thanks for testing it! It seems reiserfs with Frederic's changes 
appears to be more stable now on your system.



Yeah, thanks a lot for this testing!


 
quoted
I saw your NFS circular locking kill-the-BKL problem report on LKML 
- also attached below.

Hopefully someone on the Cc: list with NFS experience can point out 
the BKL assumption that is causing this.

	Ingo

----- Forwarded message from Alexander Beregalov [off-list ref] -----

Date: Wed, 15 Apr 2009 22:08:01 +0400
From: Alexander Beregalov <redacted>
To: linux-kernel <redacted>,
	Ingo Molnar [off-list ref], linux-nfs@vger.kernel.org
Subject: [core/kill-the-BKL] nfs3: possible circular locking dependency

Hi

I have pulled core/kill-the-BKL on top of 2.6.30-rc2.

device: '0:18': device_add

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.30-rc2-00057-g30aa902-dirty #5
-------------------------------------------------------
mount.nfs/1740 is trying to acquire lock:
 (kernel_mutex){+.+.+.}, at: [<00000000006f32dc>] lock_kernel+0x28/0x3c

but task is already holding lock:
 (&type->s_umount_key#24/1){+.+.+.}, at: [<00000000004b88a0>] sget+0x228/0x36c

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&type->s_umount_key#24/1){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<0000000000469f5c>] down_write_nested+0x38/0x50
       [<00000000004b88a0>] sget+0x228/0x36c
       [<00000000005688fc>] nfs_get_sb+0x80c/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40

-> #0 (kernel_mutex){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<00000000006f0ebc>] mutex_lock_nested+0x48/0x380
       [<00000000006f32dc>] lock_kernel+0x28/0x3c
       [<00000000006d20ec>] rpc_wait_bit_killable+0x64/0x8c
       [<00000000006f0620>] __wait_on_bit+0x64/0xc0
       [<00000000006f06e4>] out_of_line_wait_on_bit+0x68/0x7c
       [<00000000006d2938>] __rpc_execute+0x150/0x2b4
       [<00000000006d2ac0>] rpc_execute+0x24/0x34
       [<00000000006cc338>] rpc_run_task+0x64/0x74
       [<00000000006cc474>] rpc_call_sync+0x58/0x7c
       [<00000000005717b0>] nfs3_rpc_wrapper+0x24/0xa0
       [<0000000000572024>] do_proc_get_root+0x6c/0x10c
       [<00000000005720dc>] nfs3_proc_get_root+0x18/0x5c
       [<000000000056401c>] nfs_get_root+0x34/0x17c
       [<0000000000568adc>] nfs_get_sb+0x9ec/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40



This is still the dependency between bkl and s_umount_key that has 
been reported recently. I wonder if this is not a problem in the 
fs layer. I should investigate on it.
The problem seem to be that this NFS call context:

-> #0 (kernel_mutex){+.+.+.}:
       [<00000000004776d0>] lock_acquire+0x5c/0x74
       [<00000000006f0ebc>] mutex_lock_nested+0x48/0x380
       [<00000000006f32dc>] lock_kernel+0x28/0x3c
       [<00000000006d20ec>] rpc_wait_bit_killable+0x64/0x8c
       [<00000000006f0620>] __wait_on_bit+0x64/0xc0
       [<00000000006f06e4>] out_of_line_wait_on_bit+0x68/0x7c
       [<00000000006d2938>] __rpc_execute+0x150/0x2b4
       [<00000000006d2ac0>] rpc_execute+0x24/0x34
       [<00000000006cc338>] rpc_run_task+0x64/0x74
       [<00000000006cc474>] rpc_call_sync+0x58/0x7c
       [<00000000005717b0>] nfs3_rpc_wrapper+0x24/0xa0
       [<0000000000572024>] do_proc_get_root+0x6c/0x10c
       [<00000000005720dc>] nfs3_proc_get_root+0x18/0x5c
       [<000000000056401c>] nfs_get_root+0x34/0x17c
       [<0000000000568adc>] nfs_get_sb+0x9ec/0xa7c
       [<00000000004b7ec8>] vfs_kern_mount+0x44/0xa4
       [<00000000004b7f84>] do_kern_mount+0x30/0xcc
       [<00000000004cf300>] do_mount+0x7c8/0x80c
       [<00000000004ed2a4>] compat_sys_mount+0x224/0x274
       [<0000000000406154>] linux_sparc_syscall32+0x34/0x40

Can be called with the BKL held - and then it schedule()s with the 
BKL held, creating dependencies. I did the quick hack below (a year 
ago! :-) but indeed that's probably wrong: we just drop and then 
re-acquire the BKL at a very low level - inverting the dependency 
chain.

Indeed, the problem remains if we do that :-)

It's not a problem of the NFS code, it's the probem of 
vfs_kern_mount taking the BKL.

Yes, and I think the idea of Alessio to remove the Bkl at this level
is the right way. Even though this patch is beeing discussed, I
think it opened the right direction to dig.

Maybe it would be better if nfs_get_sb() dropped the BKL (knowing 
that it's called with the BKL held) - since it does not rely on the 
BKL? Not rpc_wait_bit_killable().

I wonder if it is not dropped because it implicitly protects something else.
May be simply concurrent accesses to the superblock?

Frederic.

quoted hunk
	Ingo

-------------->
From 352e0d25def53e6b36234e4dc2083ca7f5d712a9 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <redacted>
Date: Wed, 14 May 2008 17:31:41 +0200
Subject: [PATCH] remove the BKL: restructure NFS code

the naked schedule() in rpc_wait_bit_killable() caused the BKL to
be auto-dropped in the past.

avoid the immediate hang in such code. Note that this still leaves
some other locking dependencies to be sorted out in the NFS code.

Signed-off-by: Ingo Molnar <redacted>
---
 net/sunrpc/sched.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 6eab9bf..e12e571 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -224,9 +224,15 @@ EXPORT_SYMBOL_GPL(rpc_destroy_wait_queue);
 
 static int rpc_wait_bit_killable(void *word)
 {
+	int bkl = kernel_locked();
+
 	if (fatal_signal_pending(current))
 		return -ERESTARTSYS;
+	if (bkl)
+		unlock_kernel();
 	schedule();
+	if (bkl)
+		lock_kernel();

Yeah as you said, it may not drop but invert the dependency.

 	return 0;
 }
 

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help