Re: [PATCH] slub: fix false-positive lockdep warning in free_partial()
On 02/05/2014 04:57 AM, David Rientjes wrote:
> On Tue, 4 Feb 2014, Christoph Lameter wrote:
>
>>> Although this cannot actually result in a race, because on cache
>>> destruction there should not be any concurrent frees or allocations from
>>> the cache, let's add spin_lock/unlock to free_partial() just to keep
>>> lockdep happy.
>> Please add a comment that says this in the source so we know why this was
>> added.
>>
> Makes sense since there is a comment there already saying we don't need
> the lock, then with this patch we end up taking it away. The nice thing
> is that there should be no lock contention here :)
>
> I'm not sure we need to disable irqs as in the patch, though.
I'm afraid we need:
=
[ INFO: inconsistent lock state ]
3.14.0-rc1-mm1+ #4 Tainted: GW
-
inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
modprobe/2760 [HC0[0]:SC0[0]:HE1:SE1] takes:
(&(>list_lock)->rlock){?.-...}, at: []
__kmem_cache_shutdown+0x68/0x210
{IN-HARDIRQ-W} state was registered at:
[] __lock_acquire+0x8f1/0x17f0
[] lock_acquire+0x92/0x120
[] _raw_spin_lock+0x39/0x70
[] deactivate_slab+0x26b/0x500
[] flush_cpu_slab+0x3c/0x70
[] generic_smp_call_function_single_interrupt+0x52/0xb0
[] smp_call_function_single_interrupt+0x22/0x40
[] call_function_single_interrupt+0x72/0x80
[] default_idle+0x1f/0xe0
[] arch_cpu_idle+0x26/0x30
[] cpu_startup_entry+0xa6/0x290
[] start_secondary+0x1d9/0x290
irq event stamp: 3883
hardirqs last enabled at (3883): []
mutex_lock_nested+0x2af/0x3d0
hardirqs last disabled at (3882): []
mutex_lock_nested+0x9f/0x3d0
softirqs last enabled at (3866): []
__do_softirq+0x1f2/0x330
softirqs last disabled at (3851): [] irq_exit+0xd5/0xe0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock(&(>list_lock)->rlock);
lock(&(>list_lock)->rlock);
*** DEADLOCK ***
1 lock held by modprobe/2760:
#0: (slab_mutex){+.+.+.}, at: []
kmem_cache_destroy+0x22/0xf0
stack backtrace:
CPU: 0 PID: 2760 Comm: modprobe Tainted: GW3.14.0-rc1-mm1+ #4
Hardware name:
82295780 88003af89c18 816d9633 0002
88007b2b 88003af89c68 810d1001
0001 0001 822957e8 88007b2b0840
Call Trace:
[] dump_stack+0x51/0x6e
[] print_usage_bug+0x231/0x290
[] mark_lock+0x37f/0x420
[] __lock_acquire+0x789/0x17f0
[] ? wait_for_completion+0x5b/0x120
[] ? cpumask_next_and+0x23/0x40
[] ? slab_cpuup_callback+0x120/0x120
[] ? smp_call_function_many+0x5c/0x250
[] ? __kmem_cache_shutdown+0x68/0x210
[] lock_acquire+0x92/0x120
[] ? __kmem_cache_shutdown+0x68/0x210
[] ? set_page_slub_counters+0x40/0x40
[] _raw_spin_lock+0x39/0x70
[] ? __kmem_cache_shutdown+0x68/0x210
[] __kmem_cache_shutdown+0x68/0x210
[] kmem_cache_destroy+0x43/0xf0
[] xfs_qm_exit+0x15/0x30 [xfs]
[] exit_xfs_fs+0x9/0x4e4 [xfs]
[] SyS_delete_module+0x19a/0x1f0
[] ? retint_swapgs+0x13/0x1b
[] ? trace_hardirqs_on_caller+0x105/0x1d0
[] ? trace_hardirqs_on_thunk+0x3a/0x3f
[] system_call_fastpath+0x16/0x1b
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slub: fix false-positive lockdep warning in free_partial()
On Tue, 4 Feb 2014, Christoph Lameter wrote: > > Although this cannot actually result in a race, because on cache > > destruction there should not be any concurrent frees or allocations from > > the cache, let's add spin_lock/unlock to free_partial() just to keep > > lockdep happy. > > Please add a comment that says this in the source so we know why this was > added. > Makes sense since there is a comment there already saying we don't need the lock, then with this patch we end up taking it away. The nice thing is that there should be no lock contention here :) I'm not sure we need to disable irqs as in the patch, though. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slub: fix false-positive lockdep warning in free_partial()
On Tue, 4 Feb 2014, Vladimir Davydov wrote: > Although this cannot actually result in a race, because on cache > destruction there should not be any concurrent frees or allocations from > the cache, let's add spin_lock/unlock to free_partial() just to keep > lockdep happy. Please add a comment that says this in the source so we know why this was added. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] slub: fix false-positive lockdep warning in free_partial()
Commit c65c1877bd68 ("slub: use lockdep_assert_held") requires
remove_partial() to be called with n->list_lock held, but free_partial()
called from kmem_cache_close() on cache destruction does not follow this
rule, leading to a warning:
WARNING: CPU: 0 PID: 2787 at mm/slub.c:1536
__kmem_cache_shutdown+0x1b2/0x1f0()
Modules linked in:
CPU: 0 PID: 2787 Comm: modprobe Tainted: GW3.14.0-rc1-mm1+ #1
Hardware name:
0600 88003ae1dde8 816d9583 0600
88003ae1de28 8107c107
880037ab2b00 88007c240d30 ea0001ee5280 ea0001ee52a0
Call Trace:
[] dump_stack+0x51/0x6e
[] warn_slowpath_common+0x87/0xb0
[] warn_slowpath_null+0x15/0x20
[] __kmem_cache_shutdown+0x1b2/0x1f0
[] kmem_cache_destroy+0x43/0xf0
[] xfs_destroy_zones+0x103/0x110 [xfs]
[] exit_xfs_fs+0x38/0x4e4 [xfs]
[] SyS_delete_module+0x19a/0x1f0
[] ? retint_swapgs+0x13/0x1b
[] ? trace_hardirqs_on_caller+0x105/0x1d0
[] ? trace_hardirqs_on_thunk+0x3a/0x3f
[] system_call_fastpath+0x16/0x1b
Although this cannot actually result in a race, because on cache
destruction there should not be any concurrent frees or allocations from
the cache, let's add spin_lock/unlock to free_partial() just to keep
lockdep happy.
Signed-off-by: Vladimir Davydov
---
mm/slub.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/slub.c b/mm/slub.c
index 0eeea85034c8..b1054568a76e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3191,6 +3191,7 @@ static void free_partial(struct kmem_cache *s, struct
kmem_cache_node *n)
{
struct page *page, *h;
+ spin_lock_irq(>list_lock);
list_for_each_entry_safe(page, h, >partial, lru) {
if (!page->inuse) {
remove_partial(n, page);
@@ -3200,6 +3201,7 @@ static void free_partial(struct kmem_cache *s, struct
kmem_cache_node *n)
"Objects remaining in %s on kmem_cache_close()");
}
}
+ spin_unlock_irq(>list_lock);
}
/*
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] slub: fix false-positive lockdep warning in free_partial()
Commit c65c1877bd68 (slub: use lockdep_assert_held) requires
remove_partial() to be called with n-list_lock held, but free_partial()
called from kmem_cache_close() on cache destruction does not follow this
rule, leading to a warning:
WARNING: CPU: 0 PID: 2787 at mm/slub.c:1536
__kmem_cache_shutdown+0x1b2/0x1f0()
Modules linked in:
CPU: 0 PID: 2787 Comm: modprobe Tainted: GW3.14.0-rc1-mm1+ #1
Hardware name:
0600 88003ae1dde8 816d9583 0600
88003ae1de28 8107c107
880037ab2b00 88007c240d30 ea0001ee5280 ea0001ee52a0
Call Trace:
[816d9583] dump_stack+0x51/0x6e
[8107c107] warn_slowpath_common+0x87/0xb0
[8107c145] warn_slowpath_null+0x15/0x20
[811c7fe2] __kmem_cache_shutdown+0x1b2/0x1f0
[811908d3] kmem_cache_destroy+0x43/0xf0
[a013a123] xfs_destroy_zones+0x103/0x110 [xfs]
[a0192b54] exit_xfs_fs+0x38/0x4e4 [xfs]
[811036fa] SyS_delete_module+0x19a/0x1f0
[816dfcd8] ? retint_swapgs+0x13/0x1b
[810d2125] ? trace_hardirqs_on_caller+0x105/0x1d0
[81359efe] ? trace_hardirqs_on_thunk+0x3a/0x3f
[816e8539] system_call_fastpath+0x16/0x1b
Although this cannot actually result in a race, because on cache
destruction there should not be any concurrent frees or allocations from
the cache, let's add spin_lock/unlock to free_partial() just to keep
lockdep happy.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
mm/slub.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/slub.c b/mm/slub.c
index 0eeea85034c8..b1054568a76e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3191,6 +3191,7 @@ static void free_partial(struct kmem_cache *s, struct
kmem_cache_node *n)
{
struct page *page, *h;
+ spin_lock_irq(n-list_lock);
list_for_each_entry_safe(page, h, n-partial, lru) {
if (!page-inuse) {
remove_partial(n, page);
@@ -3200,6 +3201,7 @@ static void free_partial(struct kmem_cache *s, struct
kmem_cache_node *n)
Objects remaining in %s on kmem_cache_close());
}
}
+ spin_unlock_irq(n-list_lock);
}
/*
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slub: fix false-positive lockdep warning in free_partial()
On Tue, 4 Feb 2014, Vladimir Davydov wrote: Although this cannot actually result in a race, because on cache destruction there should not be any concurrent frees or allocations from the cache, let's add spin_lock/unlock to free_partial() just to keep lockdep happy. Please add a comment that says this in the source so we know why this was added. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slub: fix false-positive lockdep warning in free_partial()
On Tue, 4 Feb 2014, Christoph Lameter wrote: Although this cannot actually result in a race, because on cache destruction there should not be any concurrent frees or allocations from the cache, let's add spin_lock/unlock to free_partial() just to keep lockdep happy. Please add a comment that says this in the source so we know why this was added. Makes sense since there is a comment there already saying we don't need the lock, then with this patch we end up taking it away. The nice thing is that there should be no lock contention here :) I'm not sure we need to disable irqs as in the patch, though. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] slub: fix false-positive lockdep warning in free_partial()
On 02/05/2014 04:57 AM, David Rientjes wrote:
On Tue, 4 Feb 2014, Christoph Lameter wrote:
Although this cannot actually result in a race, because on cache
destruction there should not be any concurrent frees or allocations from
the cache, let's add spin_lock/unlock to free_partial() just to keep
lockdep happy.
Please add a comment that says this in the source so we know why this was
added.
Makes sense since there is a comment there already saying we don't need
the lock, then with this patch we end up taking it away. The nice thing
is that there should be no lock contention here :)
I'm not sure we need to disable irqs as in the patch, though.
I'm afraid we need:
=
[ INFO: inconsistent lock state ]
3.14.0-rc1-mm1+ #4 Tainted: GW
-
inconsistent {IN-HARDIRQ-W} - {HARDIRQ-ON-W} usage.
modprobe/2760 [HC0[0]:SC0[0]:HE1:SE1] takes:
((n-list_lock)-rlock){?.-...}, at: [811c7e98]
__kmem_cache_shutdown+0x68/0x210
{IN-HARDIRQ-W} state was registered at:
[810d2e21] __lock_acquire+0x8f1/0x17f0
[810d3db2] lock_acquire+0x92/0x120
[816decc9] _raw_spin_lock+0x39/0x70
[811c54cb] deactivate_slab+0x26b/0x500
[811c7dfc] flush_cpu_slab+0x3c/0x70
[81100232] generic_smp_call_function_single_interrupt+0x52/0xb0
[810451c2] smp_call_function_single_interrupt+0x22/0x40
[816e96f2] call_function_single_interrupt+0x72/0x80
[8101f9ef] default_idle+0x1f/0xe0
[8101f346] arch_cpu_idle+0x26/0x30
[810e4766] cpu_startup_entry+0xa6/0x290
[81046129] start_secondary+0x1d9/0x290
irq event stamp: 3883
hardirqs last enabled at (3883): [816dd23f]
mutex_lock_nested+0x2af/0x3d0
hardirqs last disabled at (3882): [816dd02f]
mutex_lock_nested+0x9f/0x3d0
softirqs last enabled at (3866): [810813e2]
__do_softirq+0x1f2/0x330
softirqs last disabled at (3851): [81081675] irq_exit+0xd5/0xe0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock((n-list_lock)-rlock);
Interrupt
lock((n-list_lock)-rlock);
*** DEADLOCK ***
1 lock held by modprobe/2760:
#0: (slab_mutex){+.+.+.}, at: [811908b2]
kmem_cache_destroy+0x22/0xf0
stack backtrace:
CPU: 0 PID: 2760 Comm: modprobe Tainted: GW3.14.0-rc1-mm1+ #4
Hardware name:
82295780 88003af89c18 816d9633 0002
88007b2b 88003af89c68 810d1001
0001 0001 822957e8 88007b2b0840
Call Trace:
[816d9633] dump_stack+0x51/0x6e
[810d1001] print_usage_bug+0x231/0x290
[810d1c5f] mark_lock+0x37f/0x420
[810d2cb9] __lock_acquire+0x789/0x17f0
[816db71b] ? wait_for_completion+0x5b/0x120
[8134c4b3] ? cpumask_next_and+0x23/0x40
[811c7dc0] ? slab_cpuup_callback+0x120/0x120
[810ffd4c] ? smp_call_function_many+0x5c/0x250
[811c7e98] ? __kmem_cache_shutdown+0x68/0x210
[810d3db2] lock_acquire+0x92/0x120
[811c7e98] ? __kmem_cache_shutdown+0x68/0x210
[811c1160] ? set_page_slub_counters+0x40/0x40
[816decc9] _raw_spin_lock+0x39/0x70
[811c7e98] ? __kmem_cache_shutdown+0x68/0x210
[811c7e98] __kmem_cache_shutdown+0x68/0x210
[811908d3] kmem_cache_destroy+0x43/0xf0
[a0180455] xfs_qm_exit+0x15/0x30 [xfs]
[a018ab25] exit_xfs_fs+0x9/0x4e4 [xfs]
[811036fa] SyS_delete_module+0x19a/0x1f0
[816dfd98] ? retint_swapgs+0x13/0x1b
[810d2125] ? trace_hardirqs_on_caller+0x105/0x1d0
[81359fae] ? trace_hardirqs_on_thunk+0x3a/0x3f
[816e85f9] system_call_fastpath+0x16/0x1b
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
