| Message ID | 1358143419-13074-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 14 Jan 2013, Liu Bo wrote: > This adds a leak decoder callback so that kmem_cache_destroy() > can use to generate debugging output for the allocated objects. Interesting idea. > @@ -3787,6 +3789,9 @@ static int slab_unmergeable(struct kmem_cache *s) > if (s->ctor) > return 1; > > + if (s->decoder) > + return 1; > + > /* > * We may have set a slab to be unmergeable during bootstrap. > */ The merge processing occurs during kmem_cache_create and you are setting up the decoder field afterwards! Wont work. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> The merge processing occurs during kmem_cache_create and you are setting > up the decoder field afterwards! Wont work. In the thread I suggested providing the callback at destruction: http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg21130.html I liked that it limits accesibility of the callback to the only path that uses it. - z -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 15, 2013 at 04:30:52PM +0000, Christoph Lameter wrote: > On Mon, 14 Jan 2013, Liu Bo wrote: > > > This adds a leak decoder callback so that kmem_cache_destroy() > > can use to generate debugging output for the allocated objects. > > Interesting idea. > > > @@ -3787,6 +3789,9 @@ static int slab_unmergeable(struct kmem_cache *s) > > if (s->ctor) > > return 1; > > > > + if (s->decoder) > > + return 1; > > + > > /* > > * We may have set a slab to be unmergeable during bootstrap. > > */ > > The merge processing occurs during kmem_cache_create and you are setting > up the decoder field afterwards! Wont work. You're right, I miss the lock part. thanks, liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 15, 2013 at 09:01:05AM -0800, Zach Brown wrote: > > The merge processing occurs during kmem_cache_create and you are setting > > up the decoder field afterwards! Wont work. > > In the thread I suggested providing the callback at destruction: > > http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg21130.html > > I liked that it limits accesibility of the callback to the only path > that uses it. Well, I was trying to avoid API change, but seems we have to, I'll update the patch as your suggestion in the next version. thanks, liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Mon, Jan 14, 2013 at 8:03 AM, Liu Bo <bo.li.liu@oracle.com> wrote: > This adds a leak decoder callback so that kmem_cache_destroy() > can use to generate debugging output for the allocated objects. > > Callers like btrfs are using their own leak tracking which will > manage allocated objects in a list(or something else), this does > indeed the same thing as what slab does. So adding a callback > for leak tracking can avoid this as well as runtime overhead. > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> I'm OK with adding the hooks but I'd much rather see the reporting integrated with kmemleak. CONFIG_KMEMLEAK_LIGHTWEIGHT or something, maybe? > --- > The idea is from Zach Brown <zab@zabbo.net>. > > fs/btrfs/extent_io.c | 24 ++++++++++++++++++++++++ > fs/btrfs/extent_map.c | 12 ++++++++++++ > include/linux/slab.h | 1 + > include/linux/slab_def.h | 1 + > include/linux/slub_def.h | 1 + > mm/slab_common.c | 1 + > mm/slub.c | 5 +++++ > 7 files changed, 45 insertions(+), 0 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index bcc8dff..4954f3d 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -63,6 +63,26 @@ tree_fs_info(struct extent_io_tree *tree) > return btrfs_sb(tree->mapping->host->i_sb); > } > > +static void extent_state_leak_decoder(void *object) > +{ > + struct extent_state *state = object; > + > + printk(KERN_ERR "btrfs state leak: start %llu end %llu " > + "state %lu in tree %p refs %d\n", > + (unsigned long long)state->start, > + (unsigned long long)state->end, > + state->state, state->tree, atomic_read(&state->refs)); > +} > + > +static void extent_buffer_leak_decoder(void *object) > +{ > + struct extent_buffer *eb = object; > + > + printk(KERN_ERR "btrfs buffer leak start %llu len %lu " > + "refs %d\n", (unsigned long long)eb->start, > + eb->len, atomic_read(&eb->refs)); > +} > + > int __init extent_io_init(void) > { > extent_state_cache = kmem_cache_create("btrfs_extent_state", > @@ -71,11 +91,15 @@ int __init extent_io_init(void) > if (!extent_state_cache) > return -ENOMEM; > > + extent_state_cache->decoder = extent_state_leak_decoder; > + > extent_buffer_cache = kmem_cache_create("btrfs_extent_buffer", > sizeof(struct extent_buffer), 0, > SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD, NULL); > if (!extent_buffer_cache) > goto free_state_cache; > + > + extent_buffer_cache->decoder = extent_buffer_leak_decoder; > return 0; > > free_state_cache: > diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c > index 80370d6..d598a8d 100644 > --- a/fs/btrfs/extent_map.c > +++ b/fs/btrfs/extent_map.c > @@ -16,6 +16,16 @@ static LIST_HEAD(emaps); > static DEFINE_SPINLOCK(map_leak_lock); > #endif > > +static void extent_map_leak_decoder(void *object) > +{ > + struct extent_map *em = object; > + > + printk(KERN_ERR "btrfs ext map leak: start %llu len %llu block %llu " > + "flags %lu refs %d in tree %d compress %d\n", > + em->start, em->len, em->block_start, em->flags, > + atomic_read(&em->refs), em->in_tree, (int)em->compress_type); > +} > + > int __init extent_map_init(void) > { > extent_map_cache = kmem_cache_create("btrfs_extent_map", > @@ -23,6 +33,8 @@ int __init extent_map_init(void) > SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD, NULL); > if (!extent_map_cache) > return -ENOMEM; > + > + extent_map_cache->decoder = extent_map_leak_decoder; > return 0; > } > > diff --git a/include/linux/slab.h b/include/linux/slab.h > index 5d168d7..89a9efd 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -114,6 +114,7 @@ struct kmem_cache { > const char *name; /* Slab name for sysfs */ > int refcount; /* Use counter */ > void (*ctor)(void *); /* Called on object slot creation */ > + void (*decoder)(void *);/* Called on object slot leak detection */ > struct list_head list; /* List of all slab caches on the system */ > }; > #endif > diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h > index 8bb6e0e..7ca8309 100644 > --- a/include/linux/slab_def.h > +++ b/include/linux/slab_def.h > @@ -48,6 +48,7 @@ struct kmem_cache { > > /* constructor func */ > void (*ctor)(void *obj); > + void (*decoder)(void *obj); > > /* 4) cache creation/removal */ > const char *name; > diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h > index 9db4825..fc18af7 100644 > --- a/include/linux/slub_def.h > +++ b/include/linux/slub_def.h > @@ -93,6 +93,7 @@ struct kmem_cache { > gfp_t allocflags; /* gfp flags to use on each alloc */ > int refcount; /* Refcount for slab cache destroy */ > void (*ctor)(void *); > + void (*decoder)(void *); > int inuse; /* Offset to metadata */ > int align; /* Alignment */ > int reserved; /* Reserved bytes at the end of slabs */ > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 3f3cd97..39a0fb2 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -193,6 +193,7 @@ kmem_cache_create_memcg(struct mem_cgroup *memcg, const char *name, size_t size, > s->object_size = s->size = size; > s->align = calculate_alignment(flags, align, size); > s->ctor = ctor; > + s->decoder = NULL; > > if (memcg_register_cache(memcg, s, parent_cache)) { > kmem_cache_free(kmem_cache, s); > diff --git a/mm/slub.c b/mm/slub.c > index ba2ca53..0496a2b 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -3098,6 +3098,8 @@ static void list_slab_objects(struct kmem_cache *s, struct page *page, > for_each_object(p, s, addr, page->objects) { > > if (!test_bit(slab_index(p, s, addr), map)) { > + if (unlikely(s->decoder)) > + s->decoder(p); > printk(KERN_ERR "INFO: Object 0x%p @offset=%tu\n", > p, p - addr); > print_tracking(s, p); > @@ -3787,6 +3789,9 @@ static int slab_unmergeable(struct kmem_cache *s) > if (s->ctor) > return 1; > > + if (s->decoder) > + return 1; > + > /* > * We may have set a slab to be unmergeable during bootstrap. > */ > -- > 1.7.7.6 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index bcc8dff..4954f3d 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -63,6 +63,26 @@ tree_fs_info(struct extent_io_tree *tree) return btrfs_sb(tree->mapping->host->i_sb); } +static void extent_state_leak_decoder(void *object) +{ + struct extent_state *state = object; + + printk(KERN_ERR "btrfs state leak: start %llu end %llu " + "state %lu in tree %p refs %d\n", + (unsigned long long)state->start, + (unsigned long long)state->end, + state->state, state->tree, atomic_read(&state->refs)); +} + +static void extent_buffer_leak_decoder(void *object) +{ + struct extent_buffer *eb = object; + + printk(KERN_ERR "btrfs buffer leak start %llu len %lu " + "refs %d\n", (unsigned long long)eb->start, + eb->len, atomic_read(&eb->refs)); +} + int __init extent_io_init(void) { extent_state_cache = kmem_cache_create("btrfs_extent_state", @@ -71,11 +91,15 @@ int __init extent_io_init(void) if (!extent_state_cache) return -ENOMEM; + extent_state_cache->decoder = extent_state_leak_decoder; + extent_buffer_cache = kmem_cache_create("btrfs_extent_buffer", sizeof(struct extent_buffer), 0, SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD, NULL); if (!extent_buffer_cache) goto free_state_cache; + + extent_buffer_cache->decoder = extent_buffer_leak_decoder; return 0; free_state_cache: diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c index 80370d6..d598a8d 100644 --- a/fs/btrfs/extent_map.c +++ b/fs/btrfs/extent_map.c @@ -16,6 +16,16 @@ static LIST_HEAD(emaps); static DEFINE_SPINLOCK(map_leak_lock); #endif +static void extent_map_leak_decoder(void *object) +{ + struct extent_map *em = object; + + printk(KERN_ERR "btrfs ext map leak: start %llu len %llu block %llu " + "flags %lu refs %d in tree %d compress %d\n", + em->start, em->len, em->block_start, em->flags, + atomic_read(&em->refs), em->in_tree, (int)em->compress_type); +} + int __init extent_map_init(void) { extent_map_cache = kmem_cache_create("btrfs_extent_map", @@ -23,6 +33,8 @@ int __init extent_map_init(void) SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD, NULL); if (!extent_map_cache) return -ENOMEM; + + extent_map_cache->decoder = extent_map_leak_decoder; return 0; } diff --git a/include/linux/slab.h b/include/linux/slab.h index 5d168d7..89a9efd 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -114,6 +114,7 @@ struct kmem_cache { const char *name; /* Slab name for sysfs */ int refcount; /* Use counter */ void (*ctor)(void *); /* Called on object slot creation */ + void (*decoder)(void *);/* Called on object slot leak detection */ struct list_head list; /* List of all slab caches on the system */ }; #endif diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index 8bb6e0e..7ca8309 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -48,6 +48,7 @@ struct kmem_cache { /* constructor func */ void (*ctor)(void *obj); + void (*decoder)(void *obj); /* 4) cache creation/removal */ const char *name; diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h index 9db4825..fc18af7 100644 --- a/include/linux/slub_def.h +++ b/include/linux/slub_def.h @@ -93,6 +93,7 @@ struct kmem_cache { gfp_t allocflags; /* gfp flags to use on each alloc */ int refcount; /* Refcount for slab cache destroy */ void (*ctor)(void *); + void (*decoder)(void *); int inuse; /* Offset to metadata */ int align; /* Alignment */ int reserved; /* Reserved bytes at the end of slabs */ diff --git a/mm/slab_common.c b/mm/slab_common.c index 3f3cd97..39a0fb2 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -193,6 +193,7 @@ kmem_cache_create_memcg(struct mem_cgroup *memcg, const char *name, size_t size, s->object_size = s->size = size; s->align = calculate_alignment(flags, align, size); s->ctor = ctor; + s->decoder = NULL; if (memcg_register_cache(memcg, s, parent_cache)) { kmem_cache_free(kmem_cache, s); diff --git a/mm/slub.c b/mm/slub.c index ba2ca53..0496a2b 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -3098,6 +3098,8 @@ static void list_slab_objects(struct kmem_cache *s, struct page *page, for_each_object(p, s, addr, page->objects) { if (!test_bit(slab_index(p, s, addr), map)) { + if (unlikely(s->decoder)) + s->decoder(p); printk(KERN_ERR "INFO: Object 0x%p @offset=%tu\n", p, p - addr); print_tracking(s, p); @@ -3787,6 +3789,9 @@ static int slab_unmergeable(struct kmem_cache *s) if (s->ctor) return 1; + if (s->decoder) + return 1; + /* * We may have set a slab to be unmergeable during bootstrap. */
This adds a leak decoder callback so that kmem_cache_destroy() can use to generate debugging output for the allocated objects. Callers like btrfs are using their own leak tracking which will manage allocated objects in a list(or something else), this does indeed the same thing as what slab does. So adding a callback for leak tracking can avoid this as well as runtime overhead. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- The idea is from Zach Brown <zab@zabbo.net>. fs/btrfs/extent_io.c | 24 ++++++++++++++++++++++++ fs/btrfs/extent_map.c | 12 ++++++++++++ include/linux/slab.h | 1 + include/linux/slab_def.h | 1 + include/linux/slub_def.h | 1 + mm/slab_common.c | 1 + mm/slub.c | 5 +++++ 7 files changed, 45 insertions(+), 0 deletions(-)