Message ID | 00000142b90da700-19f6b465-ff15-4b2b-9bcd-b91d71958b7f-000000@email.amazonses.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On 12/03/2013 10:22 AM, Christoph Lameter wrote: > On Mon, 2 Dec 2013, Greg KH wrote: > >> Your release function had 2 tabs for the lines, not one. > > Ah ok. Fixed. > >>>>> Index: linux/include/linux/slub_def.h >>>>> =================================================================== >>>>> --- linux.orig/include/linux/slub_def.h 2013-12-02 13:31:07.395905824 -0600 >>>>> +++ linux/include/linux/slub_def.h 2013-12-02 13:31:07.385906101 -0600 >>>>> @@ -98,4 +98,8 @@ struct kmem_cache { >>>>> struct kmem_cache_node *node[MAX_NUMNODES]; >>>>> }; >>>>> >>>>> +#ifdef CONFIG_SYSFS >>>>> +#define SLAB_SUPPORTS_SYSFS >>>> >>>> Why even define this? Why not just use CONFIG_SYSFS? >>> >>> Because not all slab allocators currently support SYSFS and there is the >>> need to have different code now in slab_common.c depending on the >>> configuration of the allocator. >> >> But you are defining something that you only ever check once, why not >> just use CONFIG_SYSFS instead as it makes more sense, not the other way >> around. > > We cannot use CONFIG_SYSFS otherwise it would break SLAB since some of > the code modified is shared between allocators. SLAB currently does not > support sysfs. When we add that then we can get rid of the #define. > > Subject: slub: use sysfs'es release mechanism for kmem_cache > > Sysfs has a release mechanism. Use that to release the kmem_cache structure > if CONFIG_SYSFS is enabled. > > Signed-off-by: Christoph Lameter <cl@linux.com> I'm still seeing warnings with this patch applied: [ 24.900482] WARNING: CPU: 12 PID: 3654 at lib/debugobjects.c:260 debug_print_object+ 0x8d/0xb0() [ 24.900482] ODEBUG: free active (active state 0) object type: timer_list hint: delay ed_work_timer_fn+0x0/0x20 [ 24.900482] Modules linked in: [ 24.900482] CPU: 12 PID: 3654 Comm: kworker/12:1 Tainted: G W 3.13.0-rc4-n ext-20131217-sasha-00013-ga878504-dirty #4149 [ 24.900482] Workqueue: events kobject_delayed_cleanup [ 24.900482] 0000000000000104 ffff8804f429bae8 ffffffff8439501c ffffffff8555a92c [ 24.900482] ffff8804f429bb38 ffff8804f429bb28 ffffffff8112f8ac ffff8804f429bb58 [ 24.900482] ffffffff856a9413 ffff880826333530 ffffffff85c68c40 ffffffff8801bb58 [ 24.900482] Call Trace: [ 24.900482] [<ffffffff8439501c>] dump_stack+0x52/0x7f [ 24.900482] [<ffffffff8112f8ac>] warn_slowpath_common+0x8c/0xc0 [ 24.900482] [<ffffffff8112f996>] warn_slowpath_fmt+0x46/0x50 [ 24.900482] [<ffffffff81adb50d>] debug_print_object+0x8d/0xb0 [ 24.900482] [<ffffffff81153090>] ? __queue_work+0x3f0/0x3f0 [ 24.900482] [<ffffffff81adbd15>] __debug_check_no_obj_freed+0xa5/0x220 [ 24.900482] [<ffffffff832b1acb>] ? rtc_device_release+0x2b/0x40 [ 24.900482] [<ffffffff832b1acb>] ? rtc_device_release+0x2b/0x40 [ 24.900482] [<ffffffff81adbea5>] debug_check_no_obj_freed+0x15/0x20 [ 24.900482] [<ffffffff812ad54f>] kfree+0x21f/0x2e0 [ 24.900482] [<ffffffff832b1acb>] rtc_device_release+0x2b/0x40 [ 24.900482] [<ffffffff8207efd5>] device_release+0x65/0xc0 [ 24.900482] [<ffffffff81ab05e5>] kobject_cleanup+0x145/0x190 [ 24.900482] [<ffffffff81ab063d>] kobject_delayed_cleanup+0xd/0x10 [ 24.900482] [<ffffffff81153a60>] process_one_work+0x320/0x530 [ 24.900482] [<ffffffff81153940>] ? process_one_work+0x200/0x530 [ 24.900482] [<ffffffff81155fe5>] worker_thread+0x215/0x350 [ 24.900482] [<ffffffff81155dd0>] ? manage_workers+0x180/0x180 [ 24.900482] [<ffffffff8115c9c5>] kthread+0x105/0x110 [ 24.900482] [<ffffffff8115c8c0>] ? set_kthreadd_affinity+0x30/0x30 [ 24.900482] [<ffffffff843a5e7c>] ret_from_fork+0x7c/0xb0 [ 24.900482] [<ffffffff8115c8c0>] ? set_kthreadd_affinity+0x30/0x30 [ 24.900482] ---[ end trace 45529ebf79b2573e ]--- Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 17 Dec 2013, Sasha Levin wrote:
> I'm still seeing warnings with this patch applied:
Looks like this is related to some device release mechanism that frees
twice?
I do not see any kmem_cache management functions in the backtrace and
therefore would guess that this is not the same issue.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 17 Dec 2013, Sasha Levin wrote: Cc'ing RTC folks > I'm still seeing warnings with this patch applied: That's not a sl*b issue. > [ 24.900482] WARNING: CPU: 12 PID: 3654 at lib/debugobjects.c:260 > debug_print_object+ > 0x8d/0xb0() > [ 24.900482] ODEBUG: free active (active state 0) object type: timer_list > hint: delay > ed_work_timer_fn+0x0/0x20 > [ 24.900482] Modules linked in: > [ 24.900482] CPU: 12 PID: 3654 Comm: kworker/12:1 Tainted: G W > 3.13.0-rc4-n > ext-20131217-sasha-00013-ga878504-dirty #4149 > [ 24.900482] Workqueue: events kobject_delayed_cleanup > [ 24.900482] 0000000000000104 ffff8804f429bae8 ffffffff8439501c > ffffffff8555a92c > [ 24.900482] ffff8804f429bb38 ffff8804f429bb28 ffffffff8112f8ac > ffff8804f429bb58 > [ 24.900482] ffffffff856a9413 ffff880826333530 ffffffff85c68c40 > ffffffff8801bb58 > [ 24.900482] Call Trace: > [ 24.900482] [<ffffffff8439501c>] dump_stack+0x52/0x7f > [ 24.900482] [<ffffffff8112f8ac>] warn_slowpath_common+0x8c/0xc0 > [ 24.900482] [<ffffffff8112f996>] warn_slowpath_fmt+0x46/0x50 > [ 24.900482] [<ffffffff81adb50d>] debug_print_object+0x8d/0xb0 > [ 24.900482] [<ffffffff81153090>] ? __queue_work+0x3f0/0x3f0 > [ 24.900482] [<ffffffff81adbd15>] __debug_check_no_obj_freed+0xa5/0x220 > [ 24.900482] [<ffffffff832b1acb>] ? rtc_device_release+0x2b/0x40 > [ 24.900482] [<ffffffff832b1acb>] ? rtc_device_release+0x2b/0x40 > [ 24.900482] [<ffffffff81adbea5>] debug_check_no_obj_freed+0x15/0x20 > [ 24.900482] [<ffffffff812ad54f>] kfree+0x21f/0x2e0 > [ 24.900482] [<ffffffff832b1acb>] rtc_device_release+0x2b/0x40 > [ 24.900482] [<ffffffff8207efd5>] device_release+0x65/0xc0 > [ 24.900482] [<ffffffff81ab05e5>] kobject_cleanup+0x145/0x190 > [ 24.900482] [<ffffffff81ab063d>] kobject_delayed_cleanup+0xd/0x10 > [ 24.900482] [<ffffffff81153a60>] process_one_work+0x320/0x530 > [ 24.900482] [<ffffffff81153940>] ? process_one_work+0x200/0x530 > [ 24.900482] [<ffffffff81155fe5>] worker_thread+0x215/0x350 > [ 24.900482] [<ffffffff81155dd0>] ? manage_workers+0x180/0x180 > [ 24.900482] [<ffffffff8115c9c5>] kthread+0x105/0x110 > [ 24.900482] [<ffffffff8115c8c0>] ? set_kthreadd_affinity+0x30/0x30 > [ 24.900482] [<ffffffff843a5e7c>] ret_from_fork+0x7c/0xb0 > [ 24.900482] [<ffffffff8115c8c0>] ? set_kthreadd_affinity+0x30/0x30 > [ 24.900482] ---[ end trace 45529ebf79b2573e ]--- > > > Thanks, > Sasha > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/17/2013 03:37 PM, Christoph Lameter wrote: > On Tue, 17 Dec 2013, Sasha Levin wrote: > >> I'm still seeing warnings with this patch applied: > > Looks like this is related to some device release mechanism that frees > twice? > > I do not see any kmem_cache management functions in the backtrace and > therefore would guess that this is not the same issue. rgr, sorry about that - I grepped for 'timer_list' which seemed to have been in all previous kmem_cache spews and thought this one is related. If that's not the case, your patch works for me. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux/include/linux/slub_def.h =================================================================== --- linux.orig/include/linux/slub_def.h 2013-12-03 09:19:26.214666210 -0600 +++ linux/include/linux/slub_def.h 2013-12-03 09:19:26.214666210 -0600 @@ -98,4 +98,8 @@ struct kmem_cache { struct kmem_cache_node *node[MAX_NUMNODES]; }; +#ifdef CONFIG_SYSFS +#define SLAB_SUPPORTS_SYSFS +#endif + #endif /* _LINUX_SLUB_DEF_H */ Index: linux/mm/slab.h =================================================================== --- linux.orig/mm/slab.h 2013-12-03 09:19:26.214666210 -0600 +++ linux/mm/slab.h 2013-12-03 09:19:26.214666210 -0600 @@ -57,6 +57,7 @@ struct mem_cgroup; struct kmem_cache * __kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size, size_t align, unsigned long flags, void (*ctor)(void *)); +void sysfs_slab_remove(struct kmem_cache *); #else static inline struct kmem_cache * __kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size, @@ -91,6 +92,7 @@ __kmem_cache_alias(struct mem_cgroup *me #define CACHE_CREATE_MASK (SLAB_CORE_FLAGS | SLAB_DEBUG_FLAGS | SLAB_CACHE_FLAGS) int __kmem_cache_shutdown(struct kmem_cache *); +void slab_kmem_cache_release(struct kmem_cache *); struct seq_file; struct file; Index: linux/mm/slab_common.c =================================================================== --- linux.orig/mm/slab_common.c 2013-12-03 09:19:26.214666210 -0600 +++ linux/mm/slab_common.c 2013-12-03 09:19:54.373883727 -0600 @@ -251,6 +251,12 @@ kmem_cache_create(const char *name, size } EXPORT_SYMBOL(kmem_cache_create); +void slab_kmem_cache_release(struct kmem_cache *s) +{ + kfree(s->name); + kmem_cache_free(kmem_cache, s); +} + void kmem_cache_destroy(struct kmem_cache *s) { /* Destroy all the children caches if we aren't a memcg cache */ @@ -268,8 +274,12 @@ void kmem_cache_destroy(struct kmem_cach rcu_barrier(); memcg_release_cache(s); - kfree(s->name); - kmem_cache_free(kmem_cache, s); +#ifdef SLAB_SUPPORTS_SYSFS + sysfs_slab_remove(s); +#else + slab_kmem_cache_release(); + +#endif } else { list_add(&s->list, &slab_caches); mutex_unlock(&slab_mutex); Index: linux/mm/slub.c =================================================================== --- linux.orig/mm/slub.c 2013-12-03 09:19:26.214666210 -0600 +++ linux/mm/slub.c 2013-12-03 09:19:26.214666210 -0600 @@ -210,7 +210,6 @@ enum track_item { TRACK_ALLOC, TRACK_FRE #ifdef CONFIG_SYSFS static int sysfs_slab_add(struct kmem_cache *); static int sysfs_slab_alias(struct kmem_cache *, const char *); -static void sysfs_slab_remove(struct kmem_cache *); static void memcg_propagate_slab_attrs(struct kmem_cache *s); #else static inline int sysfs_slab_add(struct kmem_cache *s) { return 0; } @@ -3208,23 +3207,7 @@ static inline int kmem_cache_close(struc int __kmem_cache_shutdown(struct kmem_cache *s) { - int rc = kmem_cache_close(s); - - if (!rc) { - /* - * We do the same lock strategy around sysfs_slab_add, see - * __kmem_cache_create. Because this is pretty much the last - * operation we do and the lock will be released shortly after - * that in slab_common.c, we could just move sysfs_slab_remove - * to a later point in common code. We should do that when we - * have a common sysfs framework for all allocators. - */ - mutex_unlock(&slab_mutex); - sysfs_slab_remove(s); - mutex_lock(&slab_mutex); - } - - return rc; + return kmem_cache_close(s); } /******************************************************************** @@ -5073,6 +5056,11 @@ static void memcg_propagate_slab_attrs(s #endif } +static void kmem_cache_release(struct kobject *k) +{ + slab_kmem_cache_release(to_slab(k)); +} + static const struct sysfs_ops slab_sysfs_ops = { .show = slab_attr_show, .store = slab_attr_store, @@ -5080,6 +5068,7 @@ static const struct sysfs_ops slab_sysfs static struct kobj_type slab_ktype = { .sysfs_ops = &slab_sysfs_ops, + .release = kmem_cache_release, }; static int uevent_filter(struct kset *kset, struct kobject *kobj) @@ -5184,7 +5173,7 @@ static int sysfs_slab_add(struct kmem_ca return 0; } -static void sysfs_slab_remove(struct kmem_cache *s) +void sysfs_slab_remove(struct kmem_cache *s) { if (slab_state < FULL) /*