From: Krzysztof Helt <[email protected]>
Release rdwrlock semaphore during memory allocation.
This fixes the locked already reported here:
http://www.mail-archive.com/[email protected]/msg01389.html
The problem here is that memory allocation is done with rdwrlock
semaphore taken and the VM can get into the jfs layer taking the
rdwrlock again.
Also, the patch fixes the lockdep below. This problem is created because
the rdwrlock semaphore acquires the commit_mutex and it is called with
interrupts enabled. The interrupt may hit with the commit_mutex taken
and take the rdwrlock (again) inside the interrupt context.
=========================================================
[ INFO: possible irq lock inversion dependency detected ]
2.6.32-rc3 #99
---------------------------------------------------------
kswapd0/180 just changed the state of lock:
(&jfs_ip->rdwrlock#2){++++-.}, at: [<c02e2207>] jfs_get_block+0x47/0x280
but this lock took another, RECLAIM_FS-unsafe lock in the past:
(&jfs_ip->commit_mutex){+.+.+.}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
no locks held by kswapd0/180.
the shortest dependencies between 2nd lock and 1st lock:
-> (&jfs_ip->commit_mutex){+.+.+.} ops: 7937 {
HARDIRQ-ON-W at:
[<c0246dc6>] __lock_acquire+0x5d6/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c044c125>] mutex_lock_nested+0x55/0x280
[<c02e1d11>] jfs_commit_inode+0x61/0x120
[<c02e2075>] jfs_write_inode+0x35/0x50
[<c029c2da>] writeback_single_inode+0x1ba/0x250
[<c029ca3d>] writeback_inodes_wb+0x27d/0x3b0
[<c029cc5c>] wb_writeback+0xec/0x180
[<c029cf37>] wb_do_writeback+0x1a7/0x1d0
[<c029cf92>] bdi_writeback_task+0x32/0xa0
[<c026b16d>] bdi_start_fn+0x5d/0xb0
[<c0234e5c>] kthread+0x6c/0x80
[<c0203477>] kernel_thread_helper+0x7/0x10
SOFTIRQ-ON-W at:
[<c0246ded>] __lock_acquire+0x5fd/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c044c125>] mutex_lock_nested+0x55/0x280
[<c02e1d11>] jfs_commit_inode+0x61/0x120
[<c02e2075>] jfs_write_inode+0x35/0x50
[<c029c2da>] writeback_single_inode+0x1ba/0x250
[<c029ca3d>] writeback_inodes_wb+0x27d/0x3b0
[<c029cc5c>] wb_writeback+0xec/0x180
[<c029cf37>] wb_do_writeback+0x1a7/0x1d0
[<c029cf92>] bdi_writeback_task+0x32/0xa0
[<c026b16d>] bdi_start_fn+0x5d/0xb0
[<c0234e5c>] kthread+0x6c/0x80
[<c0203477>] kernel_thread_helper+0x7/0x10
RECLAIM_FS-ON-W at:
[<c02444f5>] mark_held_locks+0x55/0x70
[<c0244b42>] lockdep_trace_alloc+0xa2/0xd0
[<c027ed78>] kmem_cache_alloc+0x28/0x100
[<c032090b>] radix_tree_preload+0x1b/0x60
[<c025b2d1>] add_to_page_cache_locked+0x21/0xe0
[<c025b3b8>] add_to_page_cache_lru+0x28/0x70
[<c025b55a>] read_cache_page_async+0xaa/0x140
[<c025b602>] read_cache_page+0x12/0x60
[<c02fac29>] __get_metapage+0x109/0x3f0
[<c02ecb3d>] diWrite+0x16d/0x580
[<c02fe295>] txCommit+0x1b5/0x1040
[<c02e3fea>] jfs_create+0x26a/0x330
[<c028a91f>] vfs_create+0x7f/0xd0
[<c028d4a1>] do_filp_open+0x6c1/0x7e0
[<c0280711>] do_sys_open+0x51/0x120
[<c0280849>] sys_open+0x29/0x40
[<c0202c45>] syscall_call+0x7/0xb
INITIAL USE at:
[<c0246a02>] __lock_acquire+0x212/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c044c125>] mutex_lock_nested+0x55/0x280
[<c02e1d11>] jfs_commit_inode+0x61/0x120
[<c02e2075>] jfs_write_inode+0x35/0x50
[<c029c2da>] writeback_single_inode+0x1ba/0x250
[<c029ca3d>] writeback_inodes_wb+0x27d/0x3b0
[<c029cc5c>] wb_writeback+0xec/0x180
[<c029cf37>] wb_do_writeback+0x1a7/0x1d0
[<c029cf92>] bdi_writeback_task+0x32/0xa0
[<c026b16d>] bdi_start_fn+0x5d/0xb0
[<c0234e5c>] kthread+0x6c/0x80
[<c0203477>] kernel_thread_helper+0x7/0x10
}
... key at: [<c0a9523c>] __key.25521+0x0/0x8
... acquired at:
[<c02461d5>] validate_chain+0xa25/0x1040
[<c0246aca>] __lock_acquire+0x2da/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c044c125>] mutex_lock_nested+0x55/0x280
[<c02f98f9>] extAlloc+0x49/0x5c0
[<c02e23e7>] jfs_get_block+0x227/0x280
[<c02a45c0>] nobh_write_begin+0x130/0x3e0
[<c02e1c2d>] jfs_write_begin+0x3d/0x50
[<c025b9de>] generic_file_buffered_write+0xde/0x260
[<c025bf44>] __generic_file_aio_write+0x244/0x490
[<c025c1e8>] generic_file_aio_write+0x58/0xc0
[<c0281f6c>] do_sync_write+0xcc/0x110
[<c02827b6>] vfs_write+0x96/0x160
[<c0282c9d>] sys_write+0x3d/0x70
[<c0202c45>] syscall_call+0x7/0xb
-> (&jfs_ip->rdwrlock#2){++++-.} ops: 9669 {
HARDIRQ-ON-W at:
[<c0246dc6>] __lock_acquire+0x5d6/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c0238ab0>] down_write_nested+0x50/0x70
[<c02e2207>] jfs_get_block+0x47/0x280
[<c02a45c0>] nobh_write_begin+0x130/0x3e0
[<c02e1c2d>] jfs_write_begin+0x3d/0x50
[<c025b9de>] generic_file_buffered_write+0xde/0x260
[<c025bf44>] __generic_file_aio_write+0x244/0x490
[<c025c1e8>] generic_file_aio_write+0x58/0xc0
[<c0281f6c>] do_sync_write+0xcc/0x110
[<c02827b6>] vfs_write+0x96/0x160
[<c0282c9d>] sys_write+0x3d/0x70
[<c0202c45>] syscall_call+0x7/0xb
HARDIRQ-ON-R at:
[<c02469cf>] __lock_acquire+0x1df/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c0238c10>] down_read_nested+0x50/0x70
[<c02e22a6>] jfs_get_block+0xe6/0x280
[<c02a9c56>] do_mpage_readpage+0x3a6/0x510
[<c02a9ebd>] mpage_readpages+0x9d/0xd0
[<c02e1c59>] jfs_readpages+0x19/0x20
[<c0262433>] __do_page_cache_readahead+0x163/0x1f0
[<c02624e8>] ra_submit+0x28/0x40
[<c026270b>] ondemand_readahead+0x12b/0x220
[<c02628a8>] page_cache_sync_readahead+0x28/0x30
[<c025c8fa>] generic_file_aio_read+0x4ca/0x640
[<c028207c>] do_sync_read+0xcc/0x110
[<c0282914>] vfs_read+0x94/0x150
[<c0282c2d>] sys_read+0x3d/0x70
[<c0202c45>] syscall_call+0x7/0xb
SOFTIRQ-ON-W at:
[<c0246ded>] __lock_acquire+0x5fd/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c0238ab0>] down_write_nested+0x50/0x70
[<c02e2207>] jfs_get_block+0x47/0x280
[<c02a45c0>] nobh_write_begin+0x130/0x3e0
[<c02e1c2d>] jfs_write_begin+0x3d/0x50
[<c025b9de>] generic_file_buffered_write+0xde/0x260
[<c025bf44>] __generic_file_aio_write+0x244/0x490
[<c025c1e8>] generic_file_aio_write+0x58/0xc0
[<c0281f6c>] do_sync_write+0xcc/0x110
[<c02827b6>] vfs_write+0x96/0x160
[<c0282c9d>] sys_write+0x3d/0x70
[<c0202c45>] syscall_call+0x7/0xb
SOFTIRQ-ON-R at:
[<c0246efe>] __lock_acquire+0x70e/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c0238c10>] down_read_nested+0x50/0x70
[<c02e22a6>] jfs_get_block+0xe6/0x280
[<c02a9c56>] do_mpage_readpage+0x3a6/0x510
[<c02a9ebd>] mpage_readpages+0x9d/0xd0
[<c02e1c59>] jfs_readpages+0x19/0x20
[<c0262433>] __do_page_cache_readahead+0x163/0x1f0
[<c02624e8>] ra_submit+0x28/0x40
[<c026270b>] ondemand_readahead+0x12b/0x220
[<c02628a8>] page_cache_sync_readahead+0x28/0x30
[<c025c8fa>] generic_file_aio_read+0x4ca/0x640
[<c028207c>] do_sync_read+0xcc/0x110
[<c0282914>] vfs_read+0x94/0x150
[<c0282c2d>] sys_read+0x3d/0x70
[<c0202c45>] syscall_call+0x7/0xb
IN-RECLAIM_FS-W at:
[<c0246ec7>] __lock_acquire+0x6d7/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c0238ab0>] down_write_nested+0x50/0x70
[<c02e2207>] jfs_get_block+0x47/0x280
[<c02a30b3>] __block_write_full_page+0xe3/0x320
[<c02a33b7>] block_write_full_page_endio+0xc7/0xe0
[<c02a33e2>] block_write_full_page+0x12/0x20
[<c02e1c9f>] jfs_writepage+0xf/0x20
[<c0264f0c>] shrink_page_list+0x31c/0x6c0
[<c0265539>] shrink_list+0x289/0x630
[<c0265b57>] shrink_zone+0x277/0x2f0
[<c026649b>] kswapd+0x48b/0x4f0
[<c0234e5c>] kthread+0x6c/0x80
[<c0203477>] kernel_thread_helper+0x7/0x10
INITIAL USE at:
[<c0246a02>] __lock_acquire+0x212/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c0238c10>] down_read_nested+0x50/0x70
[<c02e22a6>] jfs_get_block+0xe6/0x280
[<c02a9c56>] do_mpage_readpage+0x3a6/0x510
[<c02a9ebd>] mpage_readpages+0x9d/0xd0
[<c02e1c59>] jfs_readpages+0x19/0x20
[<c0262433>] __do_page_cache_readahead+0x163/0x1f0
[<c02624e8>] ra_submit+0x28/0x40
[<c026270b>] ondemand_readahead+0x12b/0x220
[<c02628a8>] page_cache_sync_readahead+0x28/0x30
[<c025c8fa>] generic_file_aio_read+0x4ca/0x640
[<c028207c>] do_sync_read+0xcc/0x110
[<c0282914>] vfs_read+0x94/0x150
[<c0282c2d>] sys_read+0x3d/0x70
[<c0202c45>] syscall_call+0x7/0xb
}
... key at: [<c0a95244>] __key.25520+0x0/0x1c
... acquired at:
[<c0247f3c>] check_usage_forwards+0x7c/0xd0
[<c024406d>] mark_lock+0x17d/0x5b0
[<c0246ec7>] __lock_acquire+0x6d7/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c0238ab0>] down_write_nested+0x50/0x70
[<c02e2207>] jfs_get_block+0x47/0x280
[<c02a30b3>] __block_write_full_page+0xe3/0x320
[<c02a33b7>] block_write_full_page_endio+0xc7/0xe0
[<c02a33e2>] block_write_full_page+0x12/0x20
[<c02e1c9f>] jfs_writepage+0xf/0x20
[<c0264f0c>] shrink_page_list+0x31c/0x6c0
[<c0265539>] shrink_list+0x289/0x630
[<c0265b57>] shrink_zone+0x277/0x2f0
[<c026649b>] kswapd+0x48b/0x4f0
[<c0234e5c>] kthread+0x6c/0x80
[<c0203477>] kernel_thread_helper+0x7/0x10
stack backtrace:
Pid: 180, comm: kswapd0 Not tainted 2.6.32-rc3 #99
Call Trace:
[<c0243d18>] print_irq_inversion_bug+0x108/0x130
[<c0247f3c>] check_usage_forwards+0x7c/0xd0
[<c024406d>] mark_lock+0x17d/0x5b0
[<c0247ec0>] ? check_usage_forwards+0x0/0xd0
[<c0246ec7>] __lock_acquire+0x6d7/0xab0
[<c024731a>] lock_acquire+0x7a/0xa0
[<c02e2207>] ? jfs_get_block+0x47/0x280
[<c0238ab0>] down_write_nested+0x50/0x70
[<c02e2207>] ? jfs_get_block+0x47/0x280
[<c02e2207>] jfs_get_block+0x47/0x280
[<c044d7dd>] ? _spin_unlock+0x1d/0x20
[<c02a17c7>] ? create_empty_buffers+0x77/0x90
[<c02a30b3>] __block_write_full_page+0xe3/0x320
[<c02e21c0>] ? jfs_get_block+0x0/0x280
[<c02a33b7>] block_write_full_page_endio+0xc7/0xe0
[<c02a3f50>] ? end_buffer_async_write+0x0/0x160
[<c02e21c0>] ? jfs_get_block+0x0/0x280
[<c024463c>] ? trace_hardirqs_on_caller+0x12c/0x180
[<c02a33e2>] block_write_full_page+0x12/0x20
[<c02a3f50>] ? end_buffer_async_write+0x0/0x160
[<c02e1c9f>] jfs_writepage+0xf/0x20
[<c0264f0c>] shrink_page_list+0x31c/0x6c0
[<c0265539>] shrink_list+0x289/0x630
[<c0265b57>] shrink_zone+0x277/0x2f0
[<c026649b>] kswapd+0x48b/0x4f0
[<c0264170>] ? isolate_pages_global+0x0/0x1f0
[<c0234f30>] ? autoremove_wake_function+0x0/0x40
[<c0266010>] ? kswapd+0x0/0x4f0
[<c0234e5c>] kthread+0x6c/0x80
[<c0234df0>] ? kthread+0x0/0x80
[<c0203477>] kernel_thread_helper+0x7/0x10
Signed-off-by: Krzysztof Helt <[email protected]>
---
I am not sure if this is the right fix to the problem. The heavy use of
the jfs volume can lock up a machine (e.g. hit me in Ubuntu 9.04).
diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index b2ae190..de18324 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -244,9 +244,22 @@ int jfs_get_block(struct inode *ip, sector_t lblock,
#ifdef _JFS_4K
if ((rc = extHint(ip, lblock64 << ip->i_sb->s_blocksize_bits, &xad)))
goto unlock;
+
+ /* release lock to avoid lockdep with jfs_ip->commit_mutex */
+ if (create)
+ IWRITE_UNLOCK(ip);
+ else
+ IREAD_UNLOCK(ip);
+
rc = extAlloc(ip, xlen, lblock64, &xad, false);
+
if (rc)
- goto unlock;
+ return rc;
+
+ if (create)
+ IWRITE_LOCK(ip, RDWRLOCK_NORMAL);
+ else
+ IREAD_LOCK(ip, RDWRLOCK_NORMAL);
set_buffer_new(bh_result);
map_bh(bh_result, ip->i_sb, addressXAD(&xad));
----------------------------------------------------
25 pa?dziernika na stadionie Polonii w Warszawie
b?d? z bia?o-czerwonymi. Zas?uguj? na to...
Zobacz wi?cej: http://klik.wp.pl/?adr=http%3A%2F%2Fcorto.www.wp.pl%2Fas%2Frugby.html&sid=892
On Tue, 2009-10-20 at 20:48 +0200, Krzysztof Helt wrote:
> From: Krzysztof Helt <[email protected]>
>
> Release rdwrlock semaphore during memory allocation.
> This fixes the locked already reported here:
>
> http://www.mail-archive.com/[email protected]/msg01389.html
>
> The problem here is that memory allocation is done with rdwrlock
> semaphore taken and the VM can get into the jfs layer taking the
> rdwrlock again.
>
> Also, the patch fixes the lockdep below. This problem is created because
> the rdwrlock semaphore acquires the commit_mutex and it is called with
> interrupts enabled. The interrupt may hit with the commit_mutex taken
> and take the rdwrlock (again) inside the interrupt context.
The rdwrlock should never be taken in interrupt context.
> =========================================================
> [ INFO: possible irq lock inversion dependency detected ]
> 2.6.32-rc3 #99
> ---------------------------------------------------------
> kswapd0/180 just changed the state of lock:
> (&jfs_ip->rdwrlock#2){++++-.}, at: [<c02e2207>] jfs_get_block+0x47/0x280
> but this lock took another, RECLAIM_FS-unsafe lock in the past:
> (&jfs_ip->commit_mutex){+.+.+.}
>
> and interrupts could create inverse lock ordering between them.
>
>
> other info that might help us debug this:
> no locks held by kswapd0/180.
>
> the shortest dependencies between 2nd lock and 1st lock:
> -> (&jfs_ip->commit_mutex){+.+.+.} ops: 7937 {
> HARDIRQ-ON-W at:
<snip>
> ---
>
> I am not sure if this is the right fix to the problem. The heavy use of
> the jfs volume can lock up a machine (e.g. hit me in Ubuntu 9.04).
I don't think we can just drop the mutex, since it protects the inode's
xtree from being modified while another thread is either reading or
writing it.
I proposed another fix here:
http://bugzilla.kernel.org/show_bug.cgi?id=13613
It seems I haven't followed up and submitted it to the vfs maintainer.
Could you please give that patch a try and see if it fixes the problem
for you?
Thanks,
Shaggy
> diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
> index b2ae190..de18324 100644
> --- a/fs/jfs/inode.c
> +++ b/fs/jfs/inode.c
> @@ -244,9 +244,22 @@ int jfs_get_block(struct inode *ip, sector_t lblock,
> #ifdef _JFS_4K
> if ((rc = extHint(ip, lblock64 << ip->i_sb->s_blocksize_bits, &xad)))
> goto unlock;
> +
> + /* release lock to avoid lockdep with jfs_ip->commit_mutex */
> + if (create)
> + IWRITE_UNLOCK(ip);
> + else
> + IREAD_UNLOCK(ip);
> +
> rc = extAlloc(ip, xlen, lblock64, &xad, false);
> +
> if (rc)
> - goto unlock;
> + return rc;
> +
> + if (create)
> + IWRITE_LOCK(ip, RDWRLOCK_NORMAL);
> + else
> + IREAD_LOCK(ip, RDWRLOCK_NORMAL);
>
> set_buffer_new(bh_result);
> map_bh(bh_result, ip->i_sb, addressXAD(&xad));
--
David Kleikamp
IBM Linux Technology Center
Dnia 20-10-2009 o godz. 22:00 Dave Kleikamp napisa?(a):
> On Tue, 2009-10-20 at 20:48 +0200, Krzysztof Helt wrote:
> > From: Krzysztof Helt <[email protected]>
> >
> > Release rdwrlock semaphore during memory allocation.
> > This fixes the locked already reported here:
> >
> > http://www.mail-archive.com/[email protected]/msg01389.html
> >
> > The problem here is that memory allocation is done with rdwrlock
> > semaphore taken and the VM can get into the jfs layer taking the
> > rdwrlock again.
> >
> > Also, the patch fixes the lockdep below. This problem is created because
> > the rdwrlock semaphore acquires the commit_mutex and it is called with
> > interrupts enabled. The interrupt may hit with the commit_mutex taken
> > and take the rdwrlock (again) inside the interrupt context.
>
> The rdwrlock should never be taken in interrupt context.
>
Right. It is taken in the RECLAIM_FS context.
> > =========================================================
> > [ INFO: possible irq lock inversion dependency detected ]
> > 2.6.32-rc3 #99
> > ---------------------------------------------------------
> > kswapd0/180 just changed the state of lock:
> > (&jfs_ip->rdwrlock#2){++++-.}, at: [<c02e2207>] jfs_get_block+0x47/0x280
> > but this lock took another, RECLAIM_FS-unsafe lock in the past:
> > (&jfs_ip->commit_mutex){+.+.+.}
> >
> > and interrupts could create inverse lock ordering between them.
> >
> >
> > other info that might help us debug this:
> > no locks held by kswapd0/180.
> >
> > the shortest dependencies between 2nd lock and 1st lock:
> > -> (&jfs_ip->commit_mutex){+.+.+.} ops: 7937 {
> > HARDIRQ-ON-W at:
>
> <snip>
>
> > ---
> >
> > I am not sure if this is the right fix to the problem. The heavy use of
> > the jfs volume can lock up a machine (e.g. hit me in Ubuntu 9.04).
>
> I don't think we can just drop the mutex, since it protects the inode's
> xtree from being modified while another thread is either reading or
> writing it.
>
> I proposed another fix here:
> http://bugzilla.kernel.org/show_bug.cgi?id=13613
>
> It seems I haven't followed up and submitted it to the vfs maintainer.
> Could you please give that patch a try and see if it fixes the problem
> for you?
>
I have tested it a little and I will do some more. At least, there is no
easy way to trigger the previous lockdep (with rdwrlock only reported to
the 2.6.30). It seems your fix helps here.
However, I think it does not help with this new lockdep I have reported.
The new locked is due to the fact that commit_mutex is taken with the
rdwrlock hold and enabled interrupts. The kswapd may start and create
the inverse locking order. The kswapd takes the rdwrlock and some other
task may hold the commit_mutex. The rdwrlock is taken in jfs_get_block()
and calls the extAlloc() which takes the commit_mutex. One possible fix
is to move the commit mutex to the jfs_get_block() before the rdwrlock.
The extAlloc() is called from the jfs_get_block() only.
Kind regards,
Krzysztof
----------------------------------------------------
WYGRAJ wycieczk? kolejow? po p??nocnej Hiszpanii!
http://klik.wp.pl/?adr=http%3A%2F%2Fwww.navigeo.pl%2Fcontest%2Fidea_tour&sid=885
On Tue, 20 Oct 2009 15:00:23 -0500
Dave Kleikamp <[email protected]> wrote:
> I proposed another fix here:
> http://bugzilla.kernel.org/show_bug.cgi?id=13613
>
> It seems I haven't followed up and submitted it to the vfs maintainer.
> Could you please give that patch a try and see if it fixes the problem
> for you?
>
Your patch fixes the previously reported (old) lockdep problem. I am not able to trigger
the lockdep report while it takes a few minutes without the patch.
Tested-by: Krzysztof Helt <[email protected]>
Your fix I have tested is below:
VFS: Fix potential deadlock in __read_cache_page()
lockdep reports a potential deadlock when using jfs because
add_to_page_cache_lru() is called from __read_cache_page() with GFP_KERNEL.
Detailed lockdep output can be found at
http://bugzilla.kernel.org/show_bug.cgi?id=13613
Passing mapping_gfp_mask(mapping) instead of GFP_KERNEL fixes the problem.
Signed-off-by: Dave Kleikamp <[email protected]>
diff --git a/mm/filemap.c b/mm/filemap.c
index ccea3b6..59f5406 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1702,7 +1702,8 @@ repeat:
page = page_cache_alloc_cold(mapping);
if (!page)
return ERR_PTR(-ENOMEM);
- err = add_to_page_cache_lru(page, mapping, index, GFP_KERNEL);
+ err = add_to_page_cache_lru(page, mapping, index,
+ mapping_gfp_mask(mapping));
if (unlikely(err)) {
page_cache_release(page);
if (err == -EEXIST)