Age | Commit message (Collapse) | Author | Files | Lines |
|
Commit 1331e7a1bbe1 ("rcu: Remove _rcu_barrier() dependency on
__stop_machine()") introduced slab_mutex -> cpu_hotplug.lock dependency
through kmem_cache_destroy() -> rcu_barrier() -> _rcu_barrier() ->
get_online_cpus().
Lockdep thinks that this might actually result in ABBA deadlock,
and reports it as below:
=== [ cut here ] ===
======================================================
[ INFO: possible circular locking dependency detected ]
3.6.0-rc5-00004-g0d8ee37 #143 Not tainted
-------------------------------------------------------
kworker/u:2/40 is trying to acquire lock:
(rcu_sched_state.barrier_mutex){+.+...}, at: [<ffffffff810f2126>] _rcu_barrier+0x26/0x1e0
but task is already holding lock:
(slab_mutex){+.+.+.}, at: [<ffffffff81176e15>] kmem_cache_destroy+0x45/0xe0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (slab_mutex){+.+.+.}:
[<ffffffff810ae1e2>] validate_chain+0x632/0x720
[<ffffffff810ae5d9>] __lock_acquire+0x309/0x530
[<ffffffff810ae921>] lock_acquire+0x121/0x190
[<ffffffff8155d4cc>] __mutex_lock_common+0x5c/0x450
[<ffffffff8155d9ee>] mutex_lock_nested+0x3e/0x50
[<ffffffff81558cb5>] cpuup_callback+0x2f/0xbe
[<ffffffff81564b83>] notifier_call_chain+0x93/0x140
[<ffffffff81076f89>] __raw_notifier_call_chain+0x9/0x10
[<ffffffff8155719d>] _cpu_up+0xba/0x14e
[<ffffffff815572ed>] cpu_up+0xbc/0x117
[<ffffffff81ae05e3>] smp_init+0x6b/0x9f
[<ffffffff81ac47d6>] kernel_init+0x147/0x1dc
[<ffffffff8156ab44>] kernel_thread_helper+0x4/0x10
-> #1 (cpu_hotplug.lock){+.+.+.}:
[<ffffffff810ae1e2>] validate_chain+0x632/0x720
[<ffffffff810ae5d9>] __lock_acquire+0x309/0x530
[<ffffffff810ae921>] lock_acquire+0x121/0x190
[<ffffffff8155d4cc>] __mutex_lock_common+0x5c/0x450
[<ffffffff8155d9ee>] mutex_lock_nested+0x3e/0x50
[<ffffffff81049197>] get_online_cpus+0x37/0x50
[<ffffffff810f21bb>] _rcu_barrier+0xbb/0x1e0
[<ffffffff810f22f0>] rcu_barrier_sched+0x10/0x20
[<ffffffff810f2309>] rcu_barrier+0x9/0x10
[<ffffffff8118c129>] deactivate_locked_super+0x49/0x90
[<ffffffff8118cc01>] deactivate_super+0x61/0x70
[<ffffffff811aaaa7>] mntput_no_expire+0x127/0x180
[<ffffffff811ab49e>] sys_umount+0x6e/0xd0
[<ffffffff81569979>] system_call_fastpath+0x16/0x1b
-> #0 (rcu_sched_state.barrier_mutex){+.+...}:
[<ffffffff810adb4e>] check_prev_add+0x3de/0x440
[<ffffffff810ae1e2>] validate_chain+0x632/0x720
[<ffffffff810ae5d9>] __lock_acquire+0x309/0x530
[<ffffffff810ae921>] lock_acquire+0x121/0x190
[<ffffffff8155d4cc>] __mutex_lock_common+0x5c/0x450
[<ffffffff8155d9ee>] mutex_lock_nested+0x3e/0x50
[<ffffffff810f2126>] _rcu_barrier+0x26/0x1e0
[<ffffffff810f22f0>] rcu_barrier_sched+0x10/0x20
[<ffffffff810f2309>] rcu_barrier+0x9/0x10
[<ffffffff81176ea1>] kmem_cache_destroy+0xd1/0xe0
[<ffffffffa04c3154>] nf_conntrack_cleanup_net+0xe4/0x110 [nf_conntrack]
[<ffffffffa04c31aa>] nf_conntrack_cleanup+0x2a/0x70 [nf_conntrack]
[<ffffffffa04c42ce>] nf_conntrack_net_exit+0x5e/0x80 [nf_conntrack]
[<ffffffff81454b79>] ops_exit_list+0x39/0x60
[<ffffffff814551ab>] cleanup_net+0xfb/0x1b0
[<ffffffff8106917b>] process_one_work+0x26b/0x4c0
[<ffffffff81069f3e>] worker_thread+0x12e/0x320
[<ffffffff8106f73e>] kthread+0x9e/0xb0
[<ffffffff8156ab44>] kernel_thread_helper+0x4/0x10
other info that might help us debug this:
Chain exists of:
rcu_sched_state.barrier_mutex --> cpu_hotplug.lock --> slab_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(slab_mutex);
lock(cpu_hotplug.lock);
lock(slab_mutex);
lock(rcu_sched_state.barrier_mutex);
*** DEADLOCK ***
=== [ cut here ] ===
This is actually a false positive. Lockdep has no way of knowing the fact
that the ABBA can actually never happen, because of special semantics of
cpu_hotplug.refcount and its handling in cpu_hotplug_begin(); the mutual
exclusion there is not achieved through mutex, but through
cpu_hotplug.refcount.
The "neither cpu_up() nor cpu_down() will proceed past cpu_hotplug_begin()
until everyone who called get_online_cpus() will call put_online_cpus()"
semantics is totally invisible to lockdep.
This patch therefore moves the unlock of slab_mutex so that rcu_barrier()
is being called with it unlocked. It has two advantages:
- it slightly reduces hold time of slab_mutex; as it's used to protect
the cachep list, it's not necessary to hold it over kmem_cache_free()
call any more
- it silences the lockdep false positive warning, as it avoids lockdep ever
learning about slab_mutex -> cpu_hotplug.lock dependency
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Acked-by: David Rientjes <rientjes@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
This reverts commit 96d17b7be0a9849d381442030886211dbb2a7061 which
caused the following errors at boot:
[ 1.114885] kobject (ffff88001a802578): tried to init an initialized object, something is seriously wrong.
[ 1.114885] Pid: 1, comm: swapper/0 Tainted: G W 3.6.0-rc1+ #6
[ 1.114885] Call Trace:
[ 1.114885] [<ffffffff81273f37>] kobject_init+0x87/0xa0
[ 1.115555] [<ffffffff8127426a>] kobject_init_and_add+0x2a/0x90
[ 1.115555] [<ffffffff8127c870>] ? sprintf+0x40/0x50
[ 1.115555] [<ffffffff81124c60>] sysfs_slab_add+0x80/0x210
[ 1.115555] [<ffffffff81100175>] kmem_cache_create+0xa5/0x250
[ 1.115555] [<ffffffff81cf24cd>] ? md_init+0x144/0x144
[ 1.115555] [<ffffffff81cf25b6>] local_init+0xa4/0x11b
[ 1.115555] [<ffffffff81cf24e1>] dm_init+0x14/0x45
[ 1.115836] [<ffffffff810001ba>] do_one_initcall+0x3a/0x160
[ 1.116834] [<ffffffff81cc2c90>] kernel_init+0x133/0x1b7
[ 1.117835] [<ffffffff81cc25c4>] ? do_early_param+0x86/0x86
[ 1.117835] [<ffffffff8171aff4>] kernel_thread_helper+0x4/0x10
[ 1.118401] [<ffffffff81cc2b5d>] ? start_kernel+0x33f/0x33f
[ 1.119832] [<ffffffff8171aff0>] ? gs_change+0xb/0xb
[ 1.120325] ------------[ cut here ]------------
[ 1.120835] WARNING: at fs/sysfs/dir.c:536 sysfs_add_one+0xc1/0xf0()
[ 1.121437] sysfs: cannot create duplicate filename '/kernel/slab/:t-0000016'
[ 1.121831] Modules linked in:
[ 1.122138] Pid: 1, comm: swapper/0 Tainted: G W 3.6.0-rc1+ #6
[ 1.122831] Call Trace:
[ 1.123074] [<ffffffff81195ce1>] ? sysfs_add_one+0xc1/0xf0
[ 1.123833] [<ffffffff8103adfa>] warn_slowpath_common+0x7a/0xb0
[ 1.124405] [<ffffffff8103aed1>] warn_slowpath_fmt+0x41/0x50
[ 1.124832] [<ffffffff81195ce1>] sysfs_add_one+0xc1/0xf0
[ 1.125337] [<ffffffff81195eb3>] create_dir+0x73/0xd0
[ 1.125832] [<ffffffff81196221>] sysfs_create_dir+0x81/0xe0
[ 1.126363] [<ffffffff81273d3d>] kobject_add_internal+0x9d/0x210
[ 1.126832] [<ffffffff812742a3>] kobject_init_and_add+0x63/0x90
[ 1.127406] [<ffffffff81124c60>] sysfs_slab_add+0x80/0x210
[ 1.127832] [<ffffffff81100175>] kmem_cache_create+0xa5/0x250
[ 1.128384] [<ffffffff81cf24cd>] ? md_init+0x144/0x144
[ 1.128833] [<ffffffff81cf25b6>] local_init+0xa4/0x11b
[ 1.129831] [<ffffffff81cf24e1>] dm_init+0x14/0x45
[ 1.130305] [<ffffffff810001ba>] do_one_initcall+0x3a/0x160
[ 1.130831] [<ffffffff81cc2c90>] kernel_init+0x133/0x1b7
[ 1.131351] [<ffffffff81cc25c4>] ? do_early_param+0x86/0x86
[ 1.131830] [<ffffffff8171aff4>] kernel_thread_helper+0x4/0x10
[ 1.132392] [<ffffffff81cc2b5d>] ? start_kernel+0x33f/0x33f
[ 1.132830] [<ffffffff8171aff0>] ? gs_change+0xb/0xb
[ 1.133315] ---[ end trace 2703540871c8fab7 ]---
[ 1.133830] ------------[ cut here ]------------
[ 1.134274] WARNING: at lib/kobject.c:196 kobject_add_internal+0x1f5/0x210()
[ 1.134829] kobject_add_internal failed for :t-0000016 with -EEXIST, don't try to register things with the same name in the same directory.
[ 1.135829] Modules linked in:
[ 1.136135] Pid: 1, comm: swapper/0 Tainted: G W 3.6.0-rc1+ #6
[ 1.136828] Call Trace:
[ 1.137071] [<ffffffff81273e95>] ? kobject_add_internal+0x1f5/0x210
[ 1.137830] [<ffffffff8103adfa>] warn_slowpath_common+0x7a/0xb0
[ 1.138402] [<ffffffff8103aed1>] warn_slowpath_fmt+0x41/0x50
[ 1.138830] [<ffffffff811955a3>] ? release_sysfs_dirent+0x73/0xf0
[ 1.139419] [<ffffffff81273e95>] kobject_add_internal+0x1f5/0x210
[ 1.139830] [<ffffffff812742a3>] kobject_init_and_add+0x63/0x90
[ 1.140429] [<ffffffff81124c60>] sysfs_slab_add+0x80/0x210
[ 1.140830] [<ffffffff81100175>] kmem_cache_create+0xa5/0x250
[ 1.141829] [<ffffffff81cf24cd>] ? md_init+0x144/0x144
[ 1.142307] [<ffffffff81cf25b6>] local_init+0xa4/0x11b
[ 1.142829] [<ffffffff81cf24e1>] dm_init+0x14/0x45
[ 1.143307] [<ffffffff810001ba>] do_one_initcall+0x3a/0x160
[ 1.143829] [<ffffffff81cc2c90>] kernel_init+0x133/0x1b7
[ 1.144352] [<ffffffff81cc25c4>] ? do_early_param+0x86/0x86
[ 1.144829] [<ffffffff8171aff4>] kernel_thread_helper+0x4/0x10
[ 1.145405] [<ffffffff81cc2b5d>] ? start_kernel+0x33f/0x33f
[ 1.145828] [<ffffffff8171aff0>] ? gs_change+0xb/0xb
[ 1.146313] ---[ end trace 2703540871c8fab8 ]---
Conflicts:
mm/slub.c
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Get rid of the refcount stuff in the allocators and do that part of
kmem_cache management in the common code.
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Do the initial settings of the fields in common code. This will allow us
to push more processing into common code later and improve readability.
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Shift the allocations to common code. That way the allocation and
freeing of the kmem_cache structures is handled by common code.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Simplify locking by moving the slab_add_sysfs after all locks have been
dropped. Eases the upcoming move to provide sysfs support for all
allocators.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
The slab aliasing logic causes some strange contortions in slub. So add
a call to deal with aliases to slab_common.c but disable it for other
slab allocators by providng stubs that fail to create aliases.
Full general support for aliases will require additional cleanup passes
and more standardization of fields in kmem_cache.
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Duping of the slabname has to be done by each slab. Moving this code to
slab_common avoids duplicate implementations.
With this patch we have common string handling for all slab allocators.
Strings passed to kmem_cache_create() are copied internally. Subsystems
can create temporary strings to create slab caches.
Slabs allocated in early states of bootstrap will never be freed (and
those can never be freed since they are essential to slab allocator
operations). During bootstrap we therefore do not have to worry about
duping names.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
What is done there can be done in __kmem_cache_shutdown.
This affects RCU handling somewhat. On rcu free all slab allocators do
not refer to other management structures than the kmem_cache structure.
Therefore these other structures can be freed before the rcu deferred
free to the page allocator occurs.
Reviewed-by: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
The freeing action is basically the same in all slab allocators.
Move to the common kmem_cache_destroy() function.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Reviewed-by: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Make all allocators use the "kmem_cache" slabname for the "kmem_cache"
structure.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Reviewed-by: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
kmem_cache_destroy does basically the same in all allocators.
Extract common code which is easy since we already have common mutex
handling.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Move the code to append the new kmem_cache to the list of slab caches to
the kmem_cache_create code in the shared code.
This is possible now since the acquisition of the mutex was moved into
kmem_cache_create().
Acked-by: David Rientjes <rientjes@google.com>
Reviewed-by: Glauber Costa <glommer@parallels.com>
Reviewed-by: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Instead of using s == NULL use an errorcode. This allows much more
detailed diagnostics as to what went wrong. As we add more functionality
from the slab allocators to the common kmem_cache_create() function we will
also add more error conditions.
Print the error code during the panic as well as in a warning if the module
can handle failure. The API for kmem_cache_create() currently does not allow
the returning of an error code. Return NULL but log the cause of the problem
in the syslog.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Acked-by: David Rientjes <rientjes@google.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
kmem_cache_create() does cache integrity checks when CONFIG_DEBUG_VM is
defined. These checks interspersed with the regular code path has lead
to compile time warnings when compiled without CONFIG_DEBUG_VM defined.
Restructuring the code to move the integrity checks in to a new function
would eliminate the current compile warning problem and also will allow
for future changes to the debug only code to evolve without introducing
new warnings in the regular path.
This restructuring work is based on the discussion in the following
thread:
https://lkml.org/lkml/2012/7/13/424
[akpm@linux-foundation.org: fix build, cleanup]
Signed-off-by: Shuah Khan <shuah.khan@hp.com>
Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
This reverts commit 455ce9eb1cfa083da0def023094190aeb133855a. Andrew
sent a better version.
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Eliminate an ifdef and a label by moving all the CONFIG_DEBUG_VM checking
inside the locked region.
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
The label oops is used in CONFIG_DEBUG_VM ifdef block and is defined
outside ifdef CONFIG_DEBUG_VM block. This results in the following
build warning when built with CONFIG_DEBUG_VM disabled. Fix to move
label oops definition to inside a CONFIG_DEBUG_VM block.
mm/slab_common.c: In function ‘kmem_cache_create’:
mm/slab_common.c:101:1: warning: label ‘oops’ defined but not used
[-Wunused-label]
Signed-off-by: Shuah Khan <shuah.khan@hp.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Move the mutex handling into the common kmem_cache_create()
function.
Then we can also move more checks out of SLAB's kmem_cache_create()
into the common code.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Use the mutex definition from SLAB and make it the common way to take a sleeping lock.
This has the effect of using a mutex instead of a rw semaphore for SLUB.
SLOB gains the use of a mutex for kmem_cache_create serialization.
Not needed now but SLOB may acquire some more features later (like slabinfo
/ sysfs support) through the expansion of the common code that will
need this.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Reviewed-by: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
All allocators have some sort of support for the bootstrap status.
Setup a common definition for the boot states and make all slab
allocators use that definition.
Reviewed-by: Glauber Costa <glommer@parallels.com>
Reviewed-by: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|
|
Kmem_cache_create() does a variety of sanity checks but those
vary depending on the allocator. Use the strictest tests and put them into
a slab_common file. Make the tests conditional on CONFIG_DEBUG_VM.
This patch has the effect of adding sanity checks for SLUB and SLOB
under CONFIG_DEBUG_VM and removes the checks in SLAB for !CONFIG_DEBUG_VM.
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
|