slab: cleanup mem hotplug synchronization
Subsystems (7)
cover slab: cleanup mem hotplug synchronization
Hi,
kmem_cache_{create,destroy,shrink} need to get a stable value of cpu/node online mask, because they init/destroy/access per-cpu/node kmem_cache parts, which can be allocated or destroyed on cpu/mem hotplug. To protect against cpu hotplug, these functions use {get,put}_online_cpus. However, they do nothing to synchronize with memory hotplug - taking the slab_mutex does not eliminate the possibility of race as described in patch 3.
What we need there is something like get_online_cpus, but for memory. We already have lock_memory_hotplug, which serves for the purpose, but it's a bit of a hammer right now, because it's backed by a mutex. As a result, it imposes some limitations to locking order, which are not desirable, and can't be used just like get_online_cpus. I propose to turn this mutex into an rw semaphore, which will be taken for reading in lock_memory_hotplug and for writing in memory hotplug code (that's what patch 1 does).
When I tried to use this rw semaphore in the slab implementation, I came across a problem with lockdep: rw_semaphore is not marked as read recursive although it is (at least, it looks so to me), so lockdep complains about wrong ordering with a sysfs internal mutex in case of slub, because in contrast to recursive read lock, non-recursive one should always be taken in the same order with a mutex. That's why in patch 2 I mark rw semaphore as read-recursive, just like rw spin lock.
Thanks,
Vladimir Davydov (3):
mem-hotplug: turn mem_hotplug_mutex to rwsem
lockdep: mark rwsem_acquire_read as recursive
slab: lock_memory_hotplug for kmem_cache_{create,destroy,shrink}
include/linux/lockdep.h | 2 +- include/linux/mmzone.h | 7 ++--- mm/memory_hotplug.c | 70 +++++++++++++++++++++-------------------------- mm/slab.c | 26 ++---------------- mm/slab.h | 1 + mm/slab_common.c | 35 ++++++++++++++++++++++-- mm/slob.c | 3 +- mm/slub.c | 5 ++-- mm/vmscan.c | 2 +- 9 files changed, 75 insertions(+), 76 deletions(-)
Signature · 7 lines
1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 1/3 mem-hotplug: turn mem_hotplug_mutex to rwsem
{un}lock_memory_hotplug is used to synchronize against memory hotplug. Currently it is backed by a mutex, which makes it a bit of hammer - threads that only want to get a stable value of online nodes mask won't be able to proceed concurrently.
This patch fixes this by turning mem_hotplug_mutex to an rw sempahore so that lock_memory_hotplug only takes it for reading while the memory hotplug code locks it for writing. This will allow to invoke code under {un}lock_memory_hotplug concurrently while still guaranteeing protection against memory hotplug.
--- include/linux/mmzone.h | 7 +++-- mm/memory_hotplug.c | 70 +++++++++++++++++++++--------------------------- mm/vmscan.c | 2 +- 3 files changed, 35 insertions(+), 44 deletions(-)
include/linux/mmzone.h +3 -4
@@ -481,9 +481,8 @@ struct zone {* give them a chance of being in the same cacheline. * * Write access to present_pages at runtime should be protected by -* lock_memory_hotplug()/unlock_memory_hotplug(). Any reader who can't -* tolerant drift of present_pages should hold memory hotplug lock to -* get a stable value. +* mem_hotplug_sem. Any reader who can't tolerant drift of +* present_pages should take it for reading to get a stable value. * * Read access to managed_pages should be safe because it's unsigned * long. Write access to zone->managed_pages and totalram_pages are
@@ -766,7 +765,7 @@ typedef struct pglist_data {nodemask_t reclaim_nodes;/* Nodes allowed to reclaim from */ wait_queue_head_t kswapd_wait;wait_queue_head_t pfmemalloc_wait; -struct task_struct *kswapd;/* Protected by lock_memory_hotplug() */ +struct task_struct *kswapd;/* Protected by mem_hotplug_sem */ int kswapd_max_order;enum zone_type classzone_idx;#ifdef CONFIG_NUMA_BALANCING
mm/memory_hotplug.c +31 -39
209 lines not on this page · raw message
mm/vmscan.c +1 -1
@@ -3418,7 +3418,7 @@ int kswapd_run(int nid)/* * Called by memory hotplug when all memory in a node is offlined. Caller must -* hold lock_memory_hotplug(). +* hold mem_hotplug_sem for writing. */ void kswapd_stop (int nid) {
Signature · 7 lines
1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/3 lockdep: mark rwsem_acquire_read as recursive
In reply to Vladimir Davydovrw_semaphore implementation allows recursing calls to down_read, but lockdep thinks that it doesn't. As a result, it will complain false-positively, e.g. if we do not observe some predefined locking order when taking an rw semaphore for reading and a mutex.
This patch makes lockdep think rw semaphore is read-recursive, just like rw spin lock.
--- include/linux/lockdep.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/lockdep.h +1 -1
@@ -500,7 +500,7 @@ static inline void print_irqtrace_events(struct task_struct *curr)#define rwsem_acquire (l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)#define rwsem_acquire_nest (l, s, t, n, i) lock_acquire_exclusive(l, s, t, n, i) -#define rwsem_acquire_read (l, s, t, i) lock_acquire_shared(l, s, t, NULL, i) +#define rwsem_acquire_read (l, s, t, i) lock_acquire_shared_recursive(l, s, t, NULL, i)#define rwsem_release (l, n, i) lock_release(l, n, i)#define lock_map_acquire (l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
Signature · 7 lines
1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Peter Zijlstra
maintainerRe: [PATCH -mm 2/3] lockdep: mark rwsem_acquire_read as recursive
Quoted 7 lines · Vladimir Davydov
rw_semaphoreimplementation allows recursing calls todown_read, but lockdep thinks that it doesn't. As a result, it will complain false-positively, e.g. if we do not observe some predefined locking order when taking an rw semaphore for reading and a mutex.This patch makes lockdep think rw semaphore is read-recursive, just like rw spin lock.
Uhm no rwsem isn't read recursive.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH -mm 2/3] lockdep: mark rwsem_acquire_read as recursive
Quoted 9 lines · Peter Zijlstra
Quoted 7 lines · Vladimir Davydov
rw_semaphoreimplementation allows recursing calls todown_read, but lockdep thinks that it doesn't. As a result, it will complain false-positively, e.g. if we do not observe some predefined locking order when taking an rw semaphore for reading and a mutex.This patch makes lockdep think rw semaphore is read-recursive, just like rw spin lock.
Uhm no rwsem isn't read recursive.
Yeah, I was mistaken and is already reworking my set. Please sorry for the noise.
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 3/3 slab: lock_memory_hotplug for kmem_cache_{create,destroy,shrink}
In reply to Vladimir DavydovWhen we create a sl[au]b cache, we allocate kmem_cache_node structures for each online NUMA node. To handle nodes taken online/offline, we register memory hotplug notifier and allocate/free kmem_cache_node corresponding to the node that changes its state for each kmem cache.
To synchronize between the two paths we hold the slab_mutex during both the cache creationg/destruction path and while tuning per-node parts of kmem caches in memory hotplug handler, but that's not quite right, because it does not guarantee that a newly created cache will have all kmem_cache_nodes initialized in case it races with memory hotplug. For instance, in case of slub:
CPU0 CPU1
---- ----
kmem_cache_create: online_pages: __kmem_cache_create: slab_memory_callback:
slab_mem_going_online_callback:
lock slab_mutex
for each slab_caches list entry
allocate kmem_cache node
unlock slab_mutex
lock slab_mutex
init_kmem_cache_nodes:
for_each_node_state(node, N_NORMAL_MEMORY)
allocate kmem_cache node
add kmem_cache to slab_caches list
unlock slab_mutex
online_pages (continued):
node_states_set_node
As a result we'll get a kmem cache with not all kmem_cache_nodes allocated.
To avoid issues like that we should hold the memory hotplug lock during the whole kmem cache creation/destruction/shrink paths, just like we do with cpu hotplug. This patch does the trick.
--- mm/slab.c | 26 ++------------------------ mm/slab.h | 1 + mm/slab_common.c | 35 +++++++++++++++++++++++++++++++++-- mm/slob.c | 3 +-- mm/slub.c | 5 ++--- 5 files changed, 39 insertions(+), 31 deletions(-)
mm/slab.c +2 -24
@@ -2442,8 +2442,7 @@ out:return nr_freed; } -/* Called with slab_mutex held to protect against cpu hotplug */ -static int __cache_shrink (struct kmem_cache *cachep) +int __kmem_cache_shrink (struct kmem_cache *cachep) {int ret =0 , i =0 ;struct kmem_cache_node *n;
@@ -2464,32 +2463,11 @@ static int __cache_shrink(struct kmem_cache *cachep)return (ret ?1 :0 ); } -/** -* kmem_cache_shrink - Shrink a cache. -* @cachep: The cache to shrink. -* -* Releases as many slabs as possible for a cache. -* To help debugging, a zero exit status indicates all slabs were released. -*/ -int kmem_cache_shrink (struct kmem_cache *cachep) -{ -int ret; -BUG_ON (!cachep ||in_interrupt ()); - -get_online_cpus (); -mutex_lock (&slab_mutex); - ret =__cache_shrink (cachep); -mutex_unlock (&slab_mutex); -put_online_cpus (); -return ret; -} -EXPORT_SYMBOL (kmem_cache_shrink); -int __kmem_cache_shutdown (struct kmem_cache *cachep) {int i;struct kmem_cache_node *n; -int rc =__cache_shrink (cachep); +int rc =__kmem_cache_shrink (cachep);if (rc)return rc;
mm/slab.h +1 -0
@@ -91,6 +91,7 @@ __kmem_cache_alias(const char *name, size_t size, size_t align,#define CACHE_CREATE_MASK (SLAB_CORE_FLAGS | SLAB_DEBUG_FLAGS | SLAB_CACHE_FLAGS)int __kmem_cache_shutdown (struct kmem_cache *); +int __kmem_cache_shrink (struct kmem_cache *);struct seq_file ;struct file ;
mm/slab_common.c +33 -2
@@ -205,6 +205,8 @@ kmem_cache_create(const char *name, size_t size, size_t align,int err;get_online_cpus (); +lock_memory_hotplug (); +mutex_lock (&slab_mutex); err =kmem_cache_sanity_check (name, size);
@@ -239,6 +241,8 @@ kmem_cache_create(const char *name, size_t size, size_t align, out_unlock:mutex_unlock (&slab_mutex); + +unlock_memory_hotplug ();put_online_cpus ();if (err) {
@@ -272,6 +276,8 @@ void kmem_cache_create_memcg(struct mem_cgroup *memcg, struct kmem_cache *root_cchar *cache_name;get_online_cpus (); +lock_memory_hotplug (); +mutex_lock (&slab_mutex);/*
@@ -299,6 +305,8 @@ void kmem_cache_create_memcg(struct mem_cgroup *memcg, struct kmem_cache *root_c out_unlock:mutex_unlock (&slab_mutex); + +unlock_memory_hotplug ();put_online_cpus (); }
@@ -326,6 +334,8 @@ static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)void kmem_cache_destroy (struct kmem_cache *s) {get_online_cpus (); +lock_memory_hotplug (); +mutex_lock (&slab_mutex); s->refcount--;
@@ -354,15 +364,36 @@ void kmem_cache_destroy(struct kmem_cache *s)memcg_free_cache_params (s);kfree (s->name);kmem_cache_free (kmem_cache, s); - goto out_put_cpus; + goto out; out_unlock:mutex_unlock (&slab_mutex); -out_put_cpus: +out: +unlock_memory_hotplug ();put_online_cpus (); }EXPORT_SYMBOL (kmem_cache_destroy); +/** +* kmem_cache_shrink - Shrink a cache. +* @cachep: The cache to shrink. +* +* Releases as many slabs as possible for a cache. +* To help debugging, a zero exit status indicates all slabs were released. +*/ +int kmem_cache_shrink (struct kmem_cache *cachep) +{ +int ret; + +get_online_cpus (); +lock_memory_hotplug (); + ret =__kmem_cache_shrink (cachep); +unlock_memory_hotplug (); +put_online_cpus (); +return ret; +} +EXPORT_SYMBOL (kmem_cache_shrink); +int slab_is_available (void ) {return slab_state >=UP ;
mm/slob.c +1 -2
@@ -620,11 +620,10 @@ int __kmem_cache_shutdown(struct kmem_cache *c)return 0 ; } -int kmem_cache_shrink (struct kmem_cache *d) +int __kmem_cache_shrink (struct kmem_cache *d) {return 0 ; } -EXPORT_SYMBOL (kmem_cache_shrink);struct kmem_cache kmem_cache_boot = { .name ="kmem_cache" ,
mm/slub.c +2 -3
@@ -3412,7 +3412,7 @@ EXPORT_SYMBOL(kfree);* being allocated from last increasing the chance that the last objects * are freed in them. */ -int kmem_cache_shrink (struct kmem_cache *s) +int __kmem_cache_shrink (struct kmem_cache *s) {int node;int i;
@@ -3468,7 +3468,6 @@ int kmem_cache_shrink(struct kmem_cache *s)kfree (slabs_by_inuse);return 0 ; } -EXPORT_SYMBOL (kmem_cache_shrink);static int slab_mem_going_offline_callback (void *arg) {
@@ -3476,7 +3475,7 @@ static int slab_mem_going_offline_callback(void *arg)mutex_lock (&slab_mutex);list_for_each_entry (s, &slab_caches, list) -kmem_cache_shrink (s); +__kmem_cache_shrink (s);mutex_unlock (&slab_mutex);return 0 ;
Signature · 7 lines
1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 8 lines · Vladimir Davydov
kmem_cache_{create,destroy,shrink} need to get a stable value of cpu/node online mask, because they
init/destroy/accessper-cpu/nodekmem_cacheparts, which can be allocated or destroyed on cpu/mem hotplug. To protect against cpu hotplug, these functions use {get,put}_online_cpus. However, they do nothing to synchronize with memory hotplug - taking theslab_mutexdoes not eliminate the possibility of race as described in patch 3.
Quoted 8 lines
What we need there is something like
get_online_cpus, but for memory. We already havelock_memory_hotplug, which serves for the purpose, but it's a bit of a hammer right now, because it's backed by a mutex. As a result, it imposes some limitations to locking order, which are not desirable, and can't be used just likeget_online_cpus. I propose to turn this mutex into an rw semaphore, which will be taken for reading inlock_memory_hotplugand for writing in memory hotplug code (that's what patch 1 does).
This is absolutely wrong, because down_read cannot be nested inside down/up_write critical section. Although it would work now, it could result in deadlocks in future. Please ignore this set completely.
Actually we need to implement a recursive rw semaphore here, just like cpu_hotplug_lock.
Sorry for the noise.
Thanks.