Hi again,
I've figured out a way to minimally modify RCU headers. If you still think
splitting RCU headers is a good idea, let me know, but this is meant to get
kmemtrace in Linus' tree as soon as possible.
I also renamed tracers as Pekka suggested.
Let me know what you think.
Cheers,
Eduard
Eduard - Gabriel Munteanu (2):
RCU: Don't include unnecessary headers, allow kmemtrace w/
tracepoints.
kmemtrace: Use tracepoints instead of markers.
include/linux/kmemtrace.h | 91 +++++++++++------------------------------
include/linux/rcuclassic.h | 1 -
include/linux/rcupdate.h | 1 -
include/linux/rcupreempt.h | 2 +-
include/linux/slab_def.h | 9 ++--
include/linux/slub_def.h | 12 ++----
lib/Kconfig.debug | 2 +-
mm/kmemtrace.c | 96 ++++++++++++++++++++++++++++++++------------
mm/slab.c | 25 +++++------
mm/slob.c | 27 +++++-------
mm/slub.c | 30 +++++--------
11 files changed, 140 insertions(+), 156 deletions(-)
linux/percpu.h includes linux/slab.h, which generates circular inclusion
dependencies when trying to switch kmemtrace to use tracepoints instead
of markers. This patch allows tracing within slab headers' inline
functions.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
include/linux/rcuclassic.h | 1 -
include/linux/rcupdate.h | 1 -
include/linux/rcupreempt.h | 2 +-
3 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/include/linux/rcuclassic.h b/include/linux/rcuclassic.h
index 5f89b62..b33004a 100644
--- a/include/linux/rcuclassic.h
+++ b/include/linux/rcuclassic.h
@@ -36,7 +36,6 @@
#include <linux/cache.h>
#include <linux/spinlock.h>
#include <linux/threads.h>
-#include <linux/percpu.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 86f1f5e..8f1c892 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -36,7 +36,6 @@
#include <linux/cache.h>
#include <linux/spinlock.h>
#include <linux/threads.h>
-#include <linux/percpu.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
#include <linux/lockdep.h>
diff --git a/include/linux/rcupreempt.h b/include/linux/rcupreempt.h
index 3e05c09..3429415 100644
--- a/include/linux/rcupreempt.h
+++ b/include/linux/rcupreempt.h
@@ -36,7 +36,7 @@
#include <linux/cache.h>
#include <linux/spinlock.h>
#include <linux/threads.h>
-#include <linux/percpu.h>
+#include <linux/smp.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
--
1.6.0.6
kmemtrace now uses tracepoints instead of markers. We no longer need to
use format specifiers. Also dropped kmemtrace_mark_alloc_node(), since
it's easy to pass -1 as the NUMA node without providing a needless and
long-named variant of the same function.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
include/linux/kmemtrace.h | 91 ++++++++++++-------------------------------
include/linux/slab_def.h | 9 ++--
include/linux/slub_def.h | 12 ++----
lib/Kconfig.debug | 2 +-
mm/kmemtrace.c | 96 ++++++++++++++++++++++++++++++++------------
mm/slab.c | 25 ++++++------
mm/slob.c | 27 +++++-------
mm/slub.c | 30 ++++++--------
8 files changed, 139 insertions(+), 153 deletions(-)
diff --git a/include/linux/kmemtrace.h b/include/linux/kmemtrace.h
index 5bea8ea..b0af88d 100644
--- a/include/linux/kmemtrace.h
+++ b/include/linux/kmemtrace.h
@@ -9,76 +9,35 @@
#ifdef __KERNEL__
+#include <linux/tracepoint.h>
#include <linux/types.h>
-#include <linux/marker.h>
-
-enum kmemtrace_type_id {
- KMEMTRACE_TYPE_KMALLOC = 0, /* kmalloc() or kfree(). */
- KMEMTRACE_TYPE_CACHE, /* kmem_cache_*(). */
- KMEMTRACE_TYPE_PAGES, /* __get_free_pages() and friends. */
-};
-
-#ifdef CONFIG_KMEMTRACE
extern void kmemtrace_init(void);
-static inline void kmemtrace_mark_alloc_node(enum kmemtrace_type_id type_id,
- unsigned long call_site,
- const void *ptr,
- size_t bytes_req,
- size_t bytes_alloc,
- gfp_t gfp_flags,
- int node)
-{
- trace_mark(kmemtrace_alloc, "type_id %d call_site %lu ptr %lu "
- "bytes_req %lu bytes_alloc %lu gfp_flags %lu node %d",
- type_id, call_site, (unsigned long) ptr,
- (unsigned long) bytes_req, (unsigned long) bytes_alloc,
- (unsigned long) gfp_flags, node);
-}
-
-static inline void kmemtrace_mark_free(enum kmemtrace_type_id type_id,
- unsigned long call_site,
- const void *ptr)
-{
- trace_mark(kmemtrace_free, "type_id %d call_site %lu ptr %lu",
- type_id, call_site, (unsigned long) ptr);
-}
-
-#else /* CONFIG_KMEMTRACE */
-
-static inline void kmemtrace_init(void)
-{
-}
-
-static inline void kmemtrace_mark_alloc_node(enum kmemtrace_type_id type_id,
- unsigned long call_site,
- const void *ptr,
- size_t bytes_req,
- size_t bytes_alloc,
- gfp_t gfp_flags,
- int node)
-{
-}
-
-static inline void kmemtrace_mark_free(enum kmemtrace_type_id type_id,
- unsigned long call_site,
- const void *ptr)
-{
-}
-
-#endif /* CONFIG_KMEMTRACE */
-
-static inline void kmemtrace_mark_alloc(enum kmemtrace_type_id type_id,
- unsigned long call_site,
- const void *ptr,
- size_t bytes_req,
- size_t bytes_alloc,
- gfp_t gfp_flags)
-{
- kmemtrace_mark_alloc_node(type_id, call_site, ptr,
- bytes_req, bytes_alloc, gfp_flags, -1);
-}
+DEFINE_TRACE(kmalloc,
+ TPPROTO(unsigned long call_site,
+ const void *ptr,
+ size_t bytes_req,
+ size_t bytes_alloc,
+ gfp_t gfp_flags,
+ int node),
+ TPARGS(call_site, ptr, bytes_req, bytes_alloc, gfp_flags, node));
+DEFINE_TRACE(kmem_cache_alloc,
+ TPPROTO(unsigned long call_site,
+ const void *ptr,
+ size_t bytes_req,
+ size_t bytes_alloc,
+ gfp_t gfp_flags,
+ int node),
+ TPARGS(call_site, ptr, bytes_req, bytes_alloc, gfp_flags, node));
+DEFINE_TRACE(kfree,
+ TPPROTO(unsigned long call_site,
+ const void *ptr),
+ TPARGS(call_site, ptr));
+DEFINE_TRACE(kmem_cache_free,
+ TPPROTO(unsigned long call_site,
+ const void *ptr),
+ TPARGS(call_site, ptr));
#endif /* __KERNEL__ */
diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index 7555ce9..fe3cea2 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -76,8 +76,8 @@ found:
ret = kmem_cache_alloc_notrace(cachep, flags);
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, _THIS_IP_, ret,
- size, slab_buffer_size(cachep), flags);
+ trace_kmalloc(_THIS_IP_, ret,
+ size, slab_buffer_size(cachep), flags, -1);
return ret;
}
@@ -134,9 +134,8 @@ found:
ret = kmem_cache_alloc_node_notrace(cachep, flags, node);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC, _THIS_IP_,
- ret, size, slab_buffer_size(cachep),
- flags, node);
+ trace_kmalloc(_THIS_IP_, ret,
+ size, slab_buffer_size(cachep), flags, node);
return ret;
}
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index dc28432..4654f14 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -220,8 +220,7 @@ static __always_inline void *kmalloc_large(size_t size, gfp_t flags)
unsigned int order = get_order(size);
void *ret = (void *) __get_free_pages(flags | __GFP_COMP, order);
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, _THIS_IP_, ret,
- size, PAGE_SIZE << order, flags);
+ trace_kmalloc(_THIS_IP_, ret, size, PAGE_SIZE << order, flags, -1);
return ret;
}
@@ -242,9 +241,8 @@ static __always_inline void *kmalloc(size_t size, gfp_t flags)
ret = kmem_cache_alloc_notrace(s, flags);
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC,
- _THIS_IP_, ret,
- size, s->size, flags);
+ trace_kmalloc(_THIS_IP_, ret,
+ size, s->size, flags, -1);
return ret;
}
@@ -283,9 +281,7 @@ static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
ret = kmem_cache_alloc_node_notrace(s, flags, node);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC,
- _THIS_IP_, ret,
- size, s->size, flags, node);
+ trace_kmalloc(_THIS_IP_, ret, size, s->size, flags, node);
return ret;
}
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b5417e2..9cf8eb8 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -805,7 +805,7 @@ config FIREWIRE_OHCI_REMOTE_DMA
config KMEMTRACE
bool "Kernel memory tracer (kmemtrace)"
- depends on RELAY && DEBUG_FS && MARKERS
+ depends on RELAY && DEBUG_FS && TRACEPOINTS
help
kmemtrace provides tracing for slab allocator functions, such as
kmalloc, kfree, kmem_cache_alloc, kmem_cache_free etc.. Collected
diff --git a/mm/kmemtrace.c b/mm/kmemtrace.c
index 2a70a80..1ff94e0 100644
--- a/mm/kmemtrace.c
+++ b/mm/kmemtrace.c
@@ -42,6 +42,12 @@ enum kmemtrace_event_id {
KMEMTRACE_EVENT_FREE,
};
+enum kmemtrace_type_id {
+ KMEMTRACE_TYPE_KMALLOC = 0, /* kmalloc() or kfree(). */
+ KMEMTRACE_TYPE_CACHE, /* kmem_cache_*(). */
+ KMEMTRACE_TYPE_PAGES, /* __get_free_pages() and friends. */
+};
+
struct kmemtrace_event {
u8 event_id;
u8 type_id;
@@ -58,8 +64,13 @@ struct kmemtrace_stats_alloc {
s32 numa_node;
} __attribute__ ((__packed__));
-static void kmemtrace_probe_alloc(void *probe_data, void *call_data,
- const char *format, va_list *args)
+static inline void kmemtrace_alloc(enum kmemtrace_type_id type_id,
+ unsigned long call_site,
+ const void *ptr,
+ size_t bytes_req,
+ size_t bytes_alloc,
+ gfp_t gfp_flags,
+ int node)
{
unsigned long flags;
struct kmemtrace_event *ev;
@@ -81,25 +92,26 @@ static void kmemtrace_probe_alloc(void *probe_data, void *call_data,
ev = buf;
ev->event_id = KMEMTRACE_EVENT_ALLOC;
- ev->type_id = va_arg(*args, int);
+ ev->type_id = type_id;
ev->event_size = sizeof(struct kmemtrace_event) +
sizeof(struct kmemtrace_stats_alloc);
ev->seq_num = atomic_add_return(1, &kmemtrace_seq_num);
- ev->call_site = va_arg(*args, unsigned long);
- ev->ptr = va_arg(*args, unsigned long);
+ ev->call_site = call_site;
+ ev->ptr = (unsigned long) ptr;
stats = buf + sizeof(struct kmemtrace_event);
- stats->bytes_req = va_arg(*args, unsigned long);
- stats->bytes_alloc = va_arg(*args, unsigned long);
- stats->gfp_flags = va_arg(*args, unsigned long);
- stats->numa_node = va_arg(*args, int);
+ stats->bytes_req = bytes_req;
+ stats->bytes_alloc = bytes_alloc;
+ stats->gfp_flags = gfp_flags;
+ stats->numa_node = node;
failed:
local_irq_restore(flags);
}
-static void kmemtrace_probe_free(void *probe_data, void *call_data,
- const char *format, va_list *args)
+static inline void kmemtrace_free(enum kmemtrace_type_id type_id,
+ unsigned long call_site,
+ const void *ptr)
{
unsigned long flags;
struct kmemtrace_event *ev;
@@ -115,16 +127,48 @@ static void kmemtrace_probe_free(void *probe_data, void *call_data,
* C99 does not guarantee the rvalues evaluation order.
*/
ev->event_id = KMEMTRACE_EVENT_FREE;
- ev->type_id = va_arg(*args, int);
+ ev->type_id = type_id;
ev->event_size = sizeof(struct kmemtrace_event);
ev->seq_num = atomic_add_return(1, &kmemtrace_seq_num);
- ev->call_site = va_arg(*args, unsigned long);
- ev->ptr = va_arg(*args, unsigned long);
+ ev->call_site = call_site;
+ ev->ptr = (unsigned long) ptr;
failed:
local_irq_restore(flags);
}
+static void kmemtrace_kmalloc(unsigned long call_site,
+ const void *ptr,
+ size_t bytes_req,
+ size_t bytes_alloc,
+ gfp_t gfp_flags,
+ int node)
+{
+ kmemtrace_alloc(KMEMTRACE_TYPE_KMALLOC, call_site, ptr,
+ bytes_req, bytes_alloc, gfp_flags, node);
+}
+
+static void kmemtrace_kmem_cache_alloc(unsigned long call_site,
+ const void *ptr,
+ size_t bytes_req,
+ size_t bytes_alloc,
+ gfp_t gfp_flags,
+ int node)
+{
+ kmemtrace_alloc(KMEMTRACE_TYPE_CACHE, call_site, ptr,
+ bytes_req, bytes_alloc, gfp_flags, node);
+}
+
+static void kmemtrace_kfree(unsigned long call_site, const void *ptr)
+{
+ kmemtrace_free(KMEMTRACE_TYPE_KMALLOC, call_site, ptr);
+}
+
+static void kmemtrace_kmem_cache_free(unsigned long call_site, const void *ptr)
+{
+ kmemtrace_free(KMEMTRACE_TYPE_CACHE, call_site, ptr);
+}
+
static struct dentry *
kmemtrace_create_buf_file(const char *filename, struct dentry *parent,
int mode, struct rchan_buf *buf, int *is_global)
@@ -173,26 +217,26 @@ static int kmemtrace_start_probes(void)
{
int err;
- err = marker_probe_register("kmemtrace_alloc", "type_id %d "
- "call_site %lu ptr %lu "
- "bytes_req %lu bytes_alloc %lu "
- "gfp_flags %lu node %d",
- kmemtrace_probe_alloc, NULL);
+ err = register_trace_kmalloc(kmemtrace_kmalloc);
+ if (err)
+ return err;
+ err = register_trace_kmem_cache_alloc(kmemtrace_kmem_cache_alloc);
+ if (err)
+ return err;
+ err = register_trace_kfree(kmemtrace_kfree);
if (err)
return err;
- err = marker_probe_register("kmemtrace_free", "type_id %d "
- "call_site %lu ptr %lu",
- kmemtrace_probe_free, NULL);
+ err = register_trace_kmem_cache_free(kmemtrace_kmem_cache_free);
return err;
}
static void kmemtrace_stop_probes(void)
{
- marker_probe_unregister("kmemtrace_alloc",
- kmemtrace_probe_alloc, NULL);
- marker_probe_unregister("kmemtrace_free",
- kmemtrace_probe_free, NULL);
+ unregister_trace_kmalloc(kmemtrace_kmalloc);
+ unregister_trace_kmem_cache_alloc(kmemtrace_kmem_cache_alloc);
+ unregister_trace_kfree(kmemtrace_kfree);
+ unregister_trace_kmem_cache_free(kmemtrace_kmem_cache_free);
}
static int kmemtrace_enabled_get(void *data, u64 *val)
diff --git a/mm/slab.c b/mm/slab.c
index 1fcb32b..505e731 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3624,8 +3624,9 @@ void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
{
void *ret = __cache_alloc(cachep, flags, __builtin_return_address(0));
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
- obj_size(cachep), cachep->buffer_size, flags);
+ trace_kmem_cache_alloc(_RET_IP_, ret,
+ obj_size(cachep), cachep->buffer_size,
+ flags, -1);
return ret;
}
@@ -3686,9 +3687,9 @@ void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
void *ret = __cache_alloc_node(cachep, flags, nodeid,
__builtin_return_address(0));
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
- obj_size(cachep), cachep->buffer_size,
- flags, nodeid);
+ trace_kmem_cache_alloc(_RET_IP_, ret,
+ obj_size(cachep), cachep->buffer_size,
+ flags, nodeid);
return ret;
}
@@ -3716,9 +3717,8 @@ __do_kmalloc_node(size_t size, gfp_t flags, int node, void *caller)
return cachep;
ret = kmem_cache_alloc_node_notrace(cachep, flags, node);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC,
- (unsigned long) caller, ret,
- size, cachep->buffer_size, flags, node);
+ trace_kmalloc((unsigned long) caller, ret,
+ size, cachep->buffer_size, flags, node);
return ret;
}
@@ -3768,9 +3768,8 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
return cachep;
ret = __cache_alloc(cachep, flags, caller);
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC,
- (unsigned long) caller, ret,
- size, cachep->buffer_size, flags);
+ trace_kmalloc((unsigned long) caller, ret,
+ size, cachep->buffer_size, flags, -1);
return ret;
}
@@ -3816,7 +3815,7 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp)
__cache_free(cachep, objp);
local_irq_restore(flags);
- kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, objp);
+ trace_kmem_cache_free(_RET_IP_, objp);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -3844,7 +3843,7 @@ void kfree(const void *objp)
__cache_free(c, (void *)objp);
local_irq_restore(flags);
- kmemtrace_mark_free(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, objp);
+ trace_kfree(_RET_IP_, objp);
}
EXPORT_SYMBOL(kfree);
diff --git a/mm/slob.c b/mm/slob.c
index 0f1a49f..5b55452 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -477,9 +477,7 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
*m = size;
ret = (void *)m + align;
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC,
- _RET_IP_, ret,
- size, size + align, gfp, node);
+ trace_kmalloc(_RET_IP_, ret, size, size + align, gfp, node);
} else {
unsigned int order = get_order(size);
@@ -490,9 +488,8 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
page->private = size;
}
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC,
- _RET_IP_, ret,
- size, PAGE_SIZE << order, gfp, node);
+ trace_kmalloc(_RET_IP_, ret,
+ size, PAGE_SIZE << order, gfp, node);
}
return ret;
@@ -514,7 +511,7 @@ void kfree(const void *block)
} else
put_page(&sp->page);
- kmemtrace_mark_free(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, block);
+ trace_kfree(_RET_IP_, block);
}
EXPORT_SYMBOL(kfree);
@@ -585,16 +582,14 @@ void *kmem_cache_alloc_node(struct kmem_cache *c, gfp_t flags, int node)
if (c->size < PAGE_SIZE) {
b = slob_alloc(c->size, flags, c->align, node);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_CACHE,
- _RET_IP_, b, c->size,
- SLOB_UNITS(c->size) * SLOB_UNIT,
- flags, node);
+ trace_kmem_cache_alloc(_RET_IP_, b, c->size,
+ SLOB_UNITS(c->size) * SLOB_UNIT,
+ flags, node);
} else {
b = slob_new_page(flags, get_order(c->size), node);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_CACHE,
- _RET_IP_, b, c->size,
- PAGE_SIZE << get_order(c->size),
- flags, node);
+ trace_kmem_cache_alloc(_RET_IP_, b, c->size,
+ PAGE_SIZE << get_order(c->size),
+ flags, node);
}
if (c->ctor)
@@ -632,7 +627,7 @@ void kmem_cache_free(struct kmem_cache *c, void *b)
__kmem_cache_free(b, c->size);
}
- kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, b);
+ trace_kmem_cache_free(_RET_IP_, b);
}
EXPORT_SYMBOL(kmem_cache_free);
diff --git a/mm/slub.c b/mm/slub.c
index f756915..bd25817 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1620,8 +1620,8 @@ void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
{
void *ret = slab_alloc(s, gfpflags, -1, _RET_IP_);
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
- s->objsize, s->size, gfpflags);
+ trace_kmem_cache_alloc(_RET_IP_, ret,
+ s->objsize, s->size, gfpflags, -1);
return ret;
}
@@ -1640,8 +1640,8 @@ void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
{
void *ret = slab_alloc(s, gfpflags, node, _RET_IP_);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
- s->objsize, s->size, gfpflags, node);
+ trace_kmem_cache_alloc(_RET_IP_, ret,
+ s->objsize, s->size, gfpflags, node);
return ret;
}
@@ -1766,7 +1766,7 @@ void kmem_cache_free(struct kmem_cache *s, void *x)
slab_free(s, page, x, _RET_IP_);
- kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, x);
+ trace_kmem_cache_free(_RET_IP_, x);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -2694,8 +2694,7 @@ void *__kmalloc(size_t size, gfp_t flags)
ret = slab_alloc(s, flags, -1, _RET_IP_);
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, ret,
- size, s->size, flags);
+ trace_kmalloc(_RET_IP_, ret, size, s->size, flags, -1);
return ret;
}
@@ -2721,10 +2720,8 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
if (unlikely(size > PAGE_SIZE)) {
ret = kmalloc_large_node(size, flags, node);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC,
- _RET_IP_, ret,
- size, PAGE_SIZE << get_order(size),
- flags, node);
+ trace_kmalloc(_RET_IP_, ret,
+ size, PAGE_SIZE << get_order(size), flags, node);
return ret;
}
@@ -2736,8 +2733,7 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
ret = slab_alloc(s, flags, node, _RET_IP_);
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, ret,
- size, s->size, flags, node);
+ trace_kmalloc(_RET_IP_, ret, size, s->size, flags, node);
return ret;
}
@@ -2798,7 +2794,7 @@ void kfree(const void *x)
}
slab_free(page->slab, page, object, _RET_IP_);
- kmemtrace_mark_free(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, x);
+ trace_kfree(_RET_IP_, x);
}
EXPORT_SYMBOL(kfree);
@@ -3272,8 +3268,7 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, unsigned long caller)
ret = slab_alloc(s, gfpflags, -1, caller);
/* Honor the call site pointer we recieved. */
- kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, caller, ret, size,
- s->size, gfpflags);
+ trace_kmalloc(caller, ret, size, s->size, gfpflags, -1);
return ret;
}
@@ -3295,8 +3290,7 @@ void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
ret = slab_alloc(s, gfpflags, node, caller);
/* Honor the call site pointer we recieved. */
- kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC, caller, ret,
- size, s->size, gfpflags, node);
+ trace_kmalloc(caller, ret, size, s->size, gfpflags, node);
return ret;
}
--
1.6.0.6
* Eduard - Gabriel Munteanu <[email protected]> wrote:
> linux/percpu.h includes linux/slab.h, which generates circular inclusion
> dependencies when trying to switch kmemtrace to use tracepoints instead
> of markers. This patch allows tracing within slab headers' inline
> functions.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> ---
> include/linux/rcuclassic.h | 1 -
> include/linux/rcupdate.h | 1 -
> include/linux/rcupreempt.h | 2 +-
> 3 files changed, 1 insertions(+), 3 deletions(-)
applied to tip/core/rcu, thanks Eduard!
I'll do some build coverage testing to see whether there's anything that
assumes the presence of percpu.h.
Ingo
On Tue, 2008-12-30 at 09:41 +0200, Eduard - Gabriel Munteanu wrote:
> kmemtrace now uses tracepoints instead of markers. We no longer need to
> use format specifiers. Also dropped kmemtrace_mark_alloc_node(), since
> it's easy to pass -1 as the NUMA node without providing a needless and
> long-named variant of the same function.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
Looks good to me.
Acked-by: Pekka Enberg <[email protected]>
* Ingo Molnar <[email protected]> wrote:
>
> * Eduard - Gabriel Munteanu <[email protected]> wrote:
>
> > linux/percpu.h includes linux/slab.h, which generates circular inclusion
> > dependencies when trying to switch kmemtrace to use tracepoints instead
> > of markers. This patch allows tracing within slab headers' inline
> > functions.
> >
> > Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> > ---
> > include/linux/rcuclassic.h | 1 -
> > include/linux/rcupdate.h | 1 -
> > include/linux/rcupreempt.h | 2 +-
> > 3 files changed, 1 insertions(+), 3 deletions(-)
>
> applied to tip/core/rcu, thanks Eduard!
>
> I'll do some build coverage testing to see whether there's anything that
> assumes the presence of percpu.h.
hm, there are quite a few such dependencies it appears, for example:
In file included from init/do_mounts_initrd.c:4:
include/linux/fs.h: In function 'alloc_secdata':
include/linux/fs.h:2214: error: implicit declaration of function 'get_zeroed_page'
include/linux/fs.h:2214: error: 'GFP_KERNEL' undeclared (first use in this function)
include/linux/fs.h:2214: error: (Each undeclared identifier is reported only once
include/linux/fs.h:2214: error: for each function it appears in.)
include/linux/fs.h: In function 'free_secdata':
include/linux/fs.h:2219: error: implicit declaration of function
'free_page'
In file included from include/linux/slab.h:13,
from include/linux/percpu.h:6,
from include/linux/percpu_counter.h:14,
from include/linux/ext2_fs_sb.h:21,
from include/linux/ext2_fs.h:70,
from init/do_mounts_initrd.c:6:
config attached.
Ingo
On Tue, Dec 30, 2008 at 08:53:37AM +0100, Ingo Molnar wrote:
> hm, there are quite a few such dependencies it appears, for example:
>
> In file included from init/do_mounts_initrd.c:4:
> include/linux/fs.h: In function 'alloc_secdata':
> include/linux/fs.h:2214: error: implicit declaration of function 'get_zeroed_page'
> include/linux/fs.h:2214: error: 'GFP_KERNEL' undeclared (first use in this function)
> include/linux/fs.h:2214: error: (Each undeclared identifier is reported only once
> include/linux/fs.h:2214: error: for each function it appears in.)
> include/linux/fs.h: In function 'free_secdata':
> include/linux/fs.h:2219: error: implicit declaration of function
> 'free_page'
> In file included from include/linux/slab.h:13,
> from include/linux/percpu.h:6,
> from include/linux/percpu_counter.h:14,
> from include/linux/ext2_fs_sb.h:21,
> from include/linux/ext2_fs.h:70,
> from init/do_mounts_initrd.c:6:
>
> config attached.
>
> Ingo
Hm, that's weird, I was also testing on a 2.6.28-based tree. And I don't
see any reference to RCU headers in this error message. My guess is some
code assumes a certain header provides some definitions it isn't
required to.
I'll checkout your tree and have a look.
Eduard
* Eduard - Gabriel Munteanu <[email protected]> wrote:
> On Tue, Dec 30, 2008 at 08:53:37AM +0100, Ingo Molnar wrote:
> > hm, there are quite a few such dependencies it appears, for example:
> >
> > In file included from init/do_mounts_initrd.c:4:
> > include/linux/fs.h: In function 'alloc_secdata':
> > include/linux/fs.h:2214: error: implicit declaration of function 'get_zeroed_page'
> > include/linux/fs.h:2214: error: 'GFP_KERNEL' undeclared (first use in this function)
> > include/linux/fs.h:2214: error: (Each undeclared identifier is reported only once
> > include/linux/fs.h:2214: error: for each function it appears in.)
> > include/linux/fs.h: In function 'free_secdata':
> > include/linux/fs.h:2219: error: implicit declaration of function
> > 'free_page'
> > In file included from include/linux/slab.h:13,
> > from include/linux/percpu.h:6,
> > from include/linux/percpu_counter.h:14,
> > from include/linux/ext2_fs_sb.h:21,
> > from include/linux/ext2_fs.h:70,
> > from init/do_mounts_initrd.c:6:
> >
> > config attached.
> >
> > Ingo
>
> Hm, that's weird, I was also testing on a 2.6.28-based tree. And I don't
> see any reference to RCU headers in this error message. My guess is some
> code assumes a certain header provides some definitions it isn't
> required to.
>
> I'll checkout your tree and have a look.
note: you'll need to apply your patch ontop of tip/master, i moved it out
of tip/master for now due to the build breakage. Let me know if you cannot
reproduce the problem, i'll then figure out what's going on.
Ingo
Em Tue, Dec 30, 2008 at 09:41:28AM +0200, Eduard - Gabriel Munteanu escreveu:
> kmemtrace now uses tracepoints instead of markers. We no longer need to
> use format specifiers. Also dropped kmemtrace_mark_alloc_node(), since
> it's easy to pass -1 as the NUMA node without providing a needless and
> long-named variant of the same function.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
> index 7555ce9..fe3cea2 100644
> --- a/include/linux/slab_def.h
> +++ b/include/linux/slab_def.h
> @@ -76,8 +76,8 @@ found:
>
> ret = kmem_cache_alloc_notrace(cachep, flags);
>
> - kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, _THIS_IP_, ret,
> - size, slab_buffer_size(cachep), flags);
> + trace_kmalloc(_THIS_IP_, ret,
> + size, slab_buffer_size(cachep), flags, -1);
Is there a way for a tracepoint to get the caller without having to pass
it explicitely?
- Arnaldo
On Tue, Dec 30, 2008 at 10:40:17AM -0200, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 30, 2008 at 09:41:28AM +0200, Eduard - Gabriel Munteanu escreveu:
> > kmemtrace now uses tracepoints instead of markers. We no longer need to
> > use format specifiers. Also dropped kmemtrace_mark_alloc_node(), since
> > it's easy to pass -1 as the NUMA node without providing a needless and
> > long-named variant of the same function.
> >
> > Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> > diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
> > index 7555ce9..fe3cea2 100644
> > --- a/include/linux/slab_def.h
> > +++ b/include/linux/slab_def.h
> > @@ -76,8 +76,8 @@ found:
> >
> > ret = kmem_cache_alloc_notrace(cachep, flags);
> >
> > - kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, _THIS_IP_, ret,
> > - size, slab_buffer_size(cachep), flags);
> > + trace_kmalloc(_THIS_IP_, ret,
> > + size, slab_buffer_size(cachep), flags, -1);
>
> Is there a way for a tracepoint to get the caller without having to pass
> it explicitely?
>
> - Arnaldo
It might be possible, but for correctness DEFINE_TRACE() should use
__always_inline instead of inline.
But it is quite pointless. Sometimes we need _RET_IP_, sometimes
_THIS_IP_ and sometimes a parameter we've been passed. That's because we
want the IP of the caller, so it depends on whether this slab function
is __always_inline, non-inlined or deeply nested within other functions
(which can be as well __always_inline or non-inlined).
Eduard
On Tue, Dec 30, 2008 at 09:41:26AM +0200, Eduard - Gabriel Munteanu wrote:
> Hi again,
>
> I've figured out a way to minimally modify RCU headers. If you still think
> splitting RCU headers is a good idea, let me know, but this is meant to get
> kmemtrace in Linus' tree as soon as possible.
I am only in favor of splitting the RCU headers if someone -really-
needs it. ;-)
So if this works, go for it!!!
Thanx, Paul
> I also renamed tracers as Pekka suggested.
>
> Let me know what you think.
>
>
> Cheers,
> Eduard
>
>
> Eduard - Gabriel Munteanu (2):
> RCU: Don't include unnecessary headers, allow kmemtrace w/
> tracepoints.
> kmemtrace: Use tracepoints instead of markers.
>
> include/linux/kmemtrace.h | 91 +++++++++++------------------------------
> include/linux/rcuclassic.h | 1 -
> include/linux/rcupdate.h | 1 -
> include/linux/rcupreempt.h | 2 +-
> include/linux/slab_def.h | 9 ++--
> include/linux/slub_def.h | 12 ++----
> lib/Kconfig.debug | 2 +-
> mm/kmemtrace.c | 96 ++++++++++++++++++++++++++++++++------------
> mm/slab.c | 25 +++++------
> mm/slob.c | 27 +++++-------
> mm/slub.c | 30 +++++--------
> 11 files changed, 140 insertions(+), 156 deletions(-)
>
* Eduard - Gabriel Munteanu ([email protected]) wrote:
> On Tue, Dec 30, 2008 at 10:40:17AM -0200, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Dec 30, 2008 at 09:41:28AM +0200, Eduard - Gabriel Munteanu escreveu:
> > > kmemtrace now uses tracepoints instead of markers. We no longer need to
> > > use format specifiers. Also dropped kmemtrace_mark_alloc_node(), since
> > > it's easy to pass -1 as the NUMA node without providing a needless and
> > > long-named variant of the same function.
> > >
> > > Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> > > diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
> > > index 7555ce9..fe3cea2 100644
> > > --- a/include/linux/slab_def.h
> > > +++ b/include/linux/slab_def.h
> > > @@ -76,8 +76,8 @@ found:
> > >
> > > ret = kmem_cache_alloc_notrace(cachep, flags);
> > >
> > > - kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, _THIS_IP_, ret,
> > > - size, slab_buffer_size(cachep), flags);
> > > + trace_kmalloc(_THIS_IP_, ret,
> > > + size, slab_buffer_size(cachep), flags, -1);
> >
> > Is there a way for a tracepoint to get the caller without having to pass
> > it explicitely?
> >
> > - Arnaldo
>
> It might be possible, but for correctness DEFINE_TRACE() should use
> __always_inline instead of inline.
>
Yes, we could (and maybe should) use __always_inline there. Hrm, which
which tree to you work ? You probably mean DECLARE_TRACE() rather than
DEFINE_TRACE() ?
I just went over your patch again. it uses the old DEFINE_TRACE() API.
You should get the latest tracepoints which have DECLARE_TRACE() (for
trace/kmemtrace.h) and then a DEFINE_TRACE() in a .c. Ah I see, you
work on 2.6.28. You should work on top of -tip, which has this new API.
Using the tracepoints present in 2.6.28 will not let you do only a
single definition of the tracepoint structure and it will lead to waste
of kernel memory by defining multiple instances of tracepoint structures
(one for each trace_*() use, so one per kmalloc()). The
Documentation/tracepoints.txt file is updated accordingly.
> But it is quite pointless. Sometimes we need _RET_IP_, sometimes
> _THIS_IP_ and sometimes a parameter we've been passed. That's because we
> want the IP of the caller, so it depends on whether this slab function
> is __always_inline, non-inlined or deeply nested within other functions
> (which can be as well __always_inline or non-inlined).
>
Hrm ? In the case we just want
trace_kmalloc(_THIS_IP, ......);
If we have __always_inline for the trace_*() declaration, isn't it the
same to just do this in the probe ?
void probe_kmalloc(......)
{
... _RET_IP_ ...;
}
This would remove a parameter from the stack created from the
instrumentation site, which is always good.
Mathieu
>
> Eduard
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Fri, Jan 02, 2009 at 04:48:05PM -0500, Mathieu Desnoyers wrote:
> Yes, we could (and maybe should) use __always_inline there. Hrm, which
> which tree to you work ? You probably mean DECLARE_TRACE() rather than
> DEFINE_TRACE() ?
>
> I just went over your patch again. it uses the old DEFINE_TRACE() API.
> You should get the latest tracepoints which have DECLARE_TRACE() (for
> trace/kmemtrace.h) and then a DEFINE_TRACE() in a .c. Ah I see, you
> work on 2.6.28. You should work on top of -tip, which has this new API.
> Using the tracepoints present in 2.6.28 will not let you do only a
> single definition of the tracepoint structure and it will lead to waste
> of kernel memory by defining multiple instances of tracepoint structures
> (one for each trace_*() use, so one per kmalloc()). The
> Documentation/tracepoints.txt file is updated accordingly.
>
I'm supposed to merge it through Ingo's tip tree. If we're talking about
the same tree, I'll do as you suggested.
> > But it is quite pointless. Sometimes we need _RET_IP_, sometimes
> > _THIS_IP_ and sometimes a parameter we've been passed. That's because we
> > want the IP of the caller, so it depends on whether this slab function
> > is __always_inline, non-inlined or deeply nested within other functions
> > (which can be as well __always_inline or non-inlined).
> >
>
> Hrm ? In the case we just want
>
> trace_kmalloc(_THIS_IP, ......);
>
> If we have __always_inline for the trace_*() declaration, isn't it the
> same to just do this in the probe ?
>
> void probe_kmalloc(......)
> {
> ... _RET_IP_ ...;
>
> }
>
> This would remove a parameter from the stack created from the
> instrumentation site, which is always good.
No, it's not always possible. Not all allocator functions (not even
those of the same kind, as in alloc/free) make up an __always_inline
chain. For example, there is one or a few instances where we pass a
value other than _RET_IP_ or _THIS_IP_ to probes. It's quite hard to
untangle things without making extensive modifications.
> Mathieu
>
> >
> > Eduard
> >
>
> --
> Mathieu Desnoyers
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
* Eduard - Gabriel Munteanu ([email protected]) wrote:
> On Fri, Jan 02, 2009 at 04:48:05PM -0500, Mathieu Desnoyers wrote:
> > Yes, we could (and maybe should) use __always_inline there. Hrm, which
> > which tree to you work ? You probably mean DECLARE_TRACE() rather than
> > DEFINE_TRACE() ?
> >
> > I just went over your patch again. it uses the old DEFINE_TRACE() API.
> > You should get the latest tracepoints which have DECLARE_TRACE() (for
> > trace/kmemtrace.h) and then a DEFINE_TRACE() in a .c. Ah I see, you
> > work on 2.6.28. You should work on top of -tip, which has this new API.
> > Using the tracepoints present in 2.6.28 will not let you do only a
> > single definition of the tracepoint structure and it will lead to waste
> > of kernel memory by defining multiple instances of tracepoint structures
> > (one for each trace_*() use, so one per kmalloc()). The
> > Documentation/tracepoints.txt file is updated accordingly.
> >
>
> I'm supposed to merge it through Ingo's tip tree. If we're talking about
> the same tree, I'll do as you suggested.
>
> > > But it is quite pointless. Sometimes we need _RET_IP_, sometimes
> > > _THIS_IP_ and sometimes a parameter we've been passed. That's because we
> > > want the IP of the caller, so it depends on whether this slab function
> > > is __always_inline, non-inlined or deeply nested within other functions
> > > (which can be as well __always_inline or non-inlined).
> > >
> >
> > Hrm ? In the case we just want
> >
> > trace_kmalloc(_THIS_IP, ......);
> >
> > If we have __always_inline for the trace_*() declaration, isn't it the
> > same to just do this in the probe ?
> >
> > void probe_kmalloc(......)
> > {
> > ... _RET_IP_ ...;
> >
> > }
> >
> > This would remove a parameter from the stack created from the
> > instrumentation site, which is always good.
>
> No, it's not always possible. Not all allocator functions (not even
> those of the same kind, as in alloc/free) make up an __always_inline
> chain. For example, there is one or a few instances where we pass a
> value other than _RET_IP_ or _THIS_IP_ to probes. It's quite hard to
> untangle things without making extensive modifications.
>
OK, so let's leave the caller ip parameter then.
Mathieu
> > Mathieu
> >
> > >
> > > Eduard
> > >
> >
> > --
> > Mathieu Desnoyers
> > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68