| Message ID | alpine.LRH.2.02.1806151817130.6333@file01.intranet.prod.int.rdu2.redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 15 Jun 2018 18:25:29 -0400 (EDT) Mikulas Patocka <mpatocka@redhat.com> wrote: > In the kernel 4.17 I removed some code from dm-bufio that did slab cache > merging (21bb13276768) - both slab and slub support merging caches with > identical attributes, so dm-bufio now just calls kmem_cache_create and > relies on implicit merging. > > This uncovered a bug in the slub subsystem - if we delete a cache and > immediatelly create another cache with the same attributes, it fails > because of duplicate filename in /sys/kernel/slab/. The slub subsystem > offloads freeing the cache to a workqueue - and if we create the new cache > before the workqueue runs, it complains because of duplicate filename in > sysfs. Huh. Surprised that such an obvious blooper survived this long. I guess a rapid del+add is uncommon. > This patch fixes the bug by moving the call of kobject_del from > sysfs_slab_remove_workfn to shutdown_cache. kobject_del must be called > while we hold slab_mutex - so that the sysfs entry is deleted before a > cache with the same attributes could be created. > > > Running device-mapper-test-suite with: Nice changelog, btw. > --- linux-2.6.orig/include/linux/slub_def.h > +++ linux-2.6/include/linux/slub_def.h > @@ -156,8 +156,12 @@ struct kmem_cache { > > #ifdef CONFIG_SYSFS > #define SLAB_SUPPORTS_SYSFS > +void sysfs_slab_unlink(struct kmem_cache *); > void sysfs_slab_release(struct kmem_cache *); > #else > +static inline void sysfs_slab_unlink(struct kmem_cache *s) > +{ > +} > static inline void sysfs_slab_release(struct kmem_cache *s) > { > } hm, that's pretty old-school. We could replace SLAB_SUPPORTS_SYSFS with CONFIG_SLAB_SUPPORTS_SYSFS, move the above logic into slab.h and.. > --- linux-2.6.orig/mm/slab_common.c > +++ linux-2.6/mm/slab_common.c > @@ -566,10 +566,14 @@ static int shutdown_cache(struct kmem_ca > list_del(&s->list); > > if (s->flags & SLAB_TYPESAFE_BY_RCU) { > +#ifdef SLAB_SUPPORTS_SYSFS > + sysfs_slab_unlink(s); > +#endif > list_add_tail(&s->list, &slab_caches_to_rcu_destroy); > schedule_work(&slab_caches_to_rcu_destroy_work); > } else { > #ifdef SLAB_SUPPORTS_SYSFS > + sysfs_slab_unlink(s); > sysfs_slab_release(s); > #else > slab_kmem_cache_release(s); remove a bunch of ifdefs. But that would be a separate thing.
Index: linux-2.6/mm/slub.c =================================================================== --- linux-2.6.orig/mm/slub.c +++ linux-2.6/mm/slub.c @@ -5694,7 +5694,6 @@ static void sysfs_slab_remove_workfn(str kset_unregister(s->memcg_kset); #endif kobject_uevent(&s->kobj, KOBJ_REMOVE); - kobject_del(&s->kobj); out: kobject_put(&s->kobj); } @@ -5779,6 +5778,12 @@ static void sysfs_slab_remove(struct kme schedule_work(&s->kobj_remove_work); } +void sysfs_slab_unlink(struct kmem_cache *s) +{ + if (slab_state >= FULL) + kobject_del(&s->kobj); +} + void sysfs_slab_release(struct kmem_cache *s) { if (slab_state >= FULL) Index: linux-2.6/include/linux/slub_def.h =================================================================== --- linux-2.6.orig/include/linux/slub_def.h +++ linux-2.6/include/linux/slub_def.h @@ -156,8 +156,12 @@ struct kmem_cache { #ifdef CONFIG_SYSFS #define SLAB_SUPPORTS_SYSFS +void sysfs_slab_unlink(struct kmem_cache *); void sysfs_slab_release(struct kmem_cache *); #else +static inline void sysfs_slab_unlink(struct kmem_cache *s) +{ +} static inline void sysfs_slab_release(struct kmem_cache *s) { } Index: linux-2.6/mm/slab_common.c =================================================================== --- linux-2.6.orig/mm/slab_common.c +++ linux-2.6/mm/slab_common.c @@ -566,10 +566,14 @@ static int shutdown_cache(struct kmem_ca list_del(&s->list); if (s->flags & SLAB_TYPESAFE_BY_RCU) { +#ifdef SLAB_SUPPORTS_SYSFS + sysfs_slab_unlink(s); +#endif list_add_tail(&s->list, &slab_caches_to_rcu_destroy); schedule_work(&slab_caches_to_rcu_destroy_work); } else { #ifdef SLAB_SUPPORTS_SYSFS + sysfs_slab_unlink(s); sysfs_slab_release(s); #else slab_kmem_cache_release(s);