This reverts commit 79cf3d5e207243eecb1c4331c569e17700fa08fa.
The reverted commit, while it fixed printk format warnings, it resulted in
marker-probe format mismatches. Another approach should be used to fix
these warnings.
---
include/linux/kmemtrace.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/kmemtrace.h b/include/linux/kmemtrace.h
index a865064..2c33201 100644
--- a/include/linux/kmemtrace.h
+++ b/include/linux/kmemtrace.h
@@ -31,7 +31,7 @@ static inline void kmemtrace_mark_alloc_node(enum kmemtrace_type_id type_id,
int node)
{
trace_mark(kmemtrace_alloc, "type_id %d call_site %lu ptr %lu "
- "bytes_req %zu bytes_alloc %zu gfp_flags %lu node %d",
+ "bytes_req %lu bytes_alloc %lu gfp_flags %lu node %d",
type_id, call_site, (unsigned long) ptr,
bytes_req, bytes_alloc, (unsigned long) gfp_flags, node);
}
--
1.5.6.1
Fix the problem "kmemtrace: fix printk format warnings" attempted to fix,
but resulted in marker-probe format mismatch warnings. Instead of carrying
size_t into probes, we get rid of it by casting to unsigned long, just as
we did with gfp_t.
This way, we don't need to change marker format strings and we don't have
to rely on other format specifiers like "%zu", making for consistent use
of more generic data types (since there are no format specifiers for
gfp_t, for example).
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
include/linux/kmemtrace.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/include/linux/kmemtrace.h b/include/linux/kmemtrace.h
index 2c33201..5bea8ea 100644
--- a/include/linux/kmemtrace.h
+++ b/include/linux/kmemtrace.h
@@ -33,7 +33,8 @@ static inline void kmemtrace_mark_alloc_node(enum kmemtrace_type_id type_id,
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,
- bytes_req, bytes_alloc, (unsigned long) gfp_flags, node);
+ (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,
--
1.5.6.1
This patch replaces __builtin_return_address(0) with _RET_IP_, since a
previous patch moved _RET_IP_ and _THIS_IP_ to include/linux/kernel.h and
they're widely available now. This makes for shorter and easier to read
code.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
mm/slub.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 4f5b961..8f66782 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1612,14 +1612,14 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
{
- return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
+ return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_alloc);
#ifdef CONFIG_NUMA
void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
{
- return slab_alloc(s, gfpflags, node, __builtin_return_address(0));
+ return slab_alloc(s, gfpflags, node, (void *) _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_alloc_node);
#endif
@@ -1730,7 +1730,7 @@ void kmem_cache_free(struct kmem_cache *s, void *x)
page = virt_to_head_page(x);
- slab_free(s, page, x, __builtin_return_address(0));
+ slab_free(s, page, x, (void *) _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -2657,7 +2657,7 @@ void *__kmalloc(size_t size, gfp_t flags)
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, -1, __builtin_return_address(0));
+ return slab_alloc(s, flags, -1, (void *) _RET_IP_);
}
EXPORT_SYMBOL(__kmalloc);
@@ -2685,7 +2685,7 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, node, __builtin_return_address(0));
+ return slab_alloc(s, flags, node, (void *) _RET_IP_);
}
EXPORT_SYMBOL(__kmalloc_node);
#endif
@@ -2742,7 +2742,7 @@ void kfree(const void *x)
put_page(page);
return;
}
- slab_free(page->slab, page, object, __builtin_return_address(0));
+ slab_free(page->slab, page, object, (void *) _RET_IP_);
}
EXPORT_SYMBOL(kfree);
--
1.5.6.1
Corrected the ABI description and the kmemtrace usage guide. Thanks to
Randy Dunlap for noticing these errors.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
Documentation/ABI/testing/debugfs-kmemtrace | 2 +-
Documentation/vm/kmemtrace.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/debugfs-kmemtrace b/Documentation/ABI/testing/debugfs-kmemtrace
index a5ff9a6..5e6a92a 100644
--- a/Documentation/ABI/testing/debugfs-kmemtrace
+++ b/Documentation/ABI/testing/debugfs-kmemtrace
@@ -63,7 +63,7 @@ Adding new data to the packet (features) is done at the end of the mandatory
data:
Feature size (2 byte)
Feature ID (1 byte)
- Feature data (Feature size - 4 bytes)
+ Feature data (Feature size - 3 bytes)
Users:
diff --git a/Documentation/vm/kmemtrace.txt b/Documentation/vm/kmemtrace.txt
index 75360b1..f656cac 100644
--- a/Documentation/vm/kmemtrace.txt
+++ b/Documentation/vm/kmemtrace.txt
@@ -61,7 +61,7 @@ III. Quick usage guide
======================
1) Get a kernel that supports kmemtrace and build it accordingly (i.e. enable
-CONFIG_KMEMTRACE and CONFIG_DEFAULT_ENABLED).
+CONFIG_KMEMTRACE and CONFIG_KMEMTRACE_DEFAULT_ENABLED).
2) Get the userspace tool and build it:
$ git-clone git://repo.or.cz/kmemtrace-user.git # current repository
--
1.5.6.1
This adds hooks for the SLUB allocator, to allow tracing with kmemtrace.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
include/linux/slub_def.h | 53 +++++++++++++++++++++++++++++++++++--
mm/slub.c | 65 +++++++++++++++++++++++++++++++++++++++++----
2 files changed, 109 insertions(+), 9 deletions(-)
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 2f5c16b..dc28432 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -10,6 +10,7 @@
#include <linux/gfp.h>
#include <linux/workqueue.h>
#include <linux/kobject.h>
+#include <linux/kmemtrace.h>
enum stat_item {
ALLOC_FASTPATH, /* Allocation from cpu slab */
@@ -204,13 +205,31 @@ static __always_inline struct kmem_cache *kmalloc_slab(size_t size)
void *kmem_cache_alloc(struct kmem_cache *, gfp_t);
void *__kmalloc(size_t size, gfp_t flags);
+#ifdef CONFIG_KMEMTRACE
+extern void *kmem_cache_alloc_notrace(struct kmem_cache *s, gfp_t gfpflags);
+#else
+static __always_inline void *
+kmem_cache_alloc_notrace(struct kmem_cache *s, gfp_t gfpflags)
+{
+ return kmem_cache_alloc(s, gfpflags);
+}
+#endif
+
static __always_inline void *kmalloc_large(size_t size, gfp_t flags)
{
- return (void *)__get_free_pages(flags | __GFP_COMP, get_order(size));
+ 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);
+
+ return ret;
}
static __always_inline void *kmalloc(size_t size, gfp_t flags)
{
+ void *ret;
+
if (__builtin_constant_p(size)) {
if (size > PAGE_SIZE)
return kmalloc_large(size, flags);
@@ -221,7 +240,13 @@ static __always_inline void *kmalloc(size_t size, gfp_t flags)
if (!s)
return ZERO_SIZE_PTR;
- return kmem_cache_alloc(s, flags);
+ ret = kmem_cache_alloc_notrace(s, flags);
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC,
+ _THIS_IP_, ret,
+ size, s->size, flags);
+
+ return ret;
}
}
return __kmalloc(size, flags);
@@ -231,8 +256,24 @@ static __always_inline void *kmalloc(size_t size, gfp_t flags)
void *__kmalloc_node(size_t size, gfp_t flags, int node);
void *kmem_cache_alloc_node(struct kmem_cache *, gfp_t flags, int node);
+#ifdef CONFIG_KMEMTRACE
+extern void *kmem_cache_alloc_node_notrace(struct kmem_cache *s,
+ gfp_t gfpflags,
+ int node);
+#else
+static __always_inline void *
+kmem_cache_alloc_node_notrace(struct kmem_cache *s,
+ gfp_t gfpflags,
+ int node)
+{
+ return kmem_cache_alloc_node(s, gfpflags, node);
+}
+#endif
+
static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
{
+ void *ret;
+
if (__builtin_constant_p(size) &&
size <= PAGE_SIZE && !(flags & SLUB_DMA)) {
struct kmem_cache *s = kmalloc_slab(size);
@@ -240,7 +281,13 @@ static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
if (!s)
return ZERO_SIZE_PTR;
- return kmem_cache_alloc_node(s, flags, 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);
+
+ return ret;
}
return __kmalloc_node(size, flags, node);
}
diff --git a/mm/slub.c b/mm/slub.c
index 8f66782..06755e2 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -23,6 +23,7 @@
#include <linux/kallsyms.h>
#include <linux/memory.h>
#include <linux/math64.h>
+#include <linux/kmemtrace.h>
/*
* Lock order:
@@ -1612,18 +1613,46 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
{
- return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
+ void *ret = slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
+ s->objsize, s->size, gfpflags);
+
+ return ret;
}
EXPORT_SYMBOL(kmem_cache_alloc);
+#ifdef CONFIG_KMEMTRACE
+void *kmem_cache_alloc_notrace(struct kmem_cache *s, gfp_t gfpflags)
+{
+ return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_notrace);
+#endif
+
#ifdef CONFIG_NUMA
void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
{
- return slab_alloc(s, gfpflags, node, (void *) _RET_IP_);
+ void *ret = slab_alloc(s, gfpflags, node, (void *) _RET_IP_);
+
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
+ s->objsize, s->size, gfpflags, node);
+
+ return ret;
}
EXPORT_SYMBOL(kmem_cache_alloc_node);
#endif
+#ifdef CONFIG_KMEMTRACE
+void *kmem_cache_alloc_node_notrace(struct kmem_cache *s,
+ gfp_t gfpflags,
+ int node)
+{
+ return slab_alloc(s, gfpflags, node, (void *) _RET_IP_);
+}
+EXPORT_SYMBOL(kmem_cache_alloc_node_notrace);
+#endif
+
/*
* Slow patch handling. This may still be called frequently since objects
* have a longer lifetime than the cpu slabs in most processing loads.
@@ -1731,6 +1760,8 @@ void kmem_cache_free(struct kmem_cache *s, void *x)
page = virt_to_head_page(x);
slab_free(s, page, x, (void *) _RET_IP_);
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, x);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -2648,6 +2679,7 @@ static struct kmem_cache *get_slab(size_t size, gfp_t flags)
void *__kmalloc(size_t size, gfp_t flags)
{
struct kmem_cache *s;
+ void *ret;
if (unlikely(size > PAGE_SIZE))
return kmalloc_large(size, flags);
@@ -2657,7 +2689,12 @@ void *__kmalloc(size_t size, gfp_t flags)
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, -1, (void *) _RET_IP_);
+ ret = slab_alloc(s, flags, -1, (void *) _RET_IP_);
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, ret,
+ size, s->size, flags);
+
+ return ret;
}
EXPORT_SYMBOL(__kmalloc);
@@ -2676,16 +2713,30 @@ static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
void *__kmalloc_node(size_t size, gfp_t flags, int node)
{
struct kmem_cache *s;
+ void *ret;
- if (unlikely(size > PAGE_SIZE))
- return kmalloc_large_node(size, flags, 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);
+
+ return ret;
+ }
s = get_slab(size, flags);
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, node, (void *) _RET_IP_);
+ ret = slab_alloc(s, flags, node, (void *) _RET_IP_);
+
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, ret,
+ size, s->size, flags, node);
+
+ return ret;
}
EXPORT_SYMBOL(__kmalloc_node);
#endif
@@ -2743,6 +2794,8 @@ void kfree(const void *x)
return;
}
slab_free(page->slab, page, object, (void *) _RET_IP_);
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_KMALLOC, _RET_IP_, x);
}
EXPORT_SYMBOL(kfree);
--
1.5.6.1
On Tue, 19 Aug 2008, Eduard - Gabriel Munteanu wrote:
> This reverts commit 79cf3d5e207243eecb1c4331c569e17700fa08fa.
>
> The reverted commit, while it fixed printk format warnings, it resulted in
> marker-probe format mismatches. Another approach should be used to fix
> these warnings.
Such as what?
Can marker probes be fixed instead?
After seeing & fixing lots of various warnings in the last few days,
I'm thinking that people don't look at/heed warnings nowadays. Sad.
Maybe there are just so many that they are lost in the noise.
> ---
> include/linux/kmemtrace.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/kmemtrace.h b/include/linux/kmemtrace.h
> index a865064..2c33201 100644
> --- a/include/linux/kmemtrace.h
> +++ b/include/linux/kmemtrace.h
> @@ -31,7 +31,7 @@ static inline void kmemtrace_mark_alloc_node(enum kmemtrace_type_id type_id,
> int node)
> {
> trace_mark(kmemtrace_alloc, "type_id %d call_site %lu ptr %lu "
> - "bytes_req %zu bytes_alloc %zu gfp_flags %lu node %d",
> + "bytes_req %lu bytes_alloc %lu gfp_flags %lu node %d",
> type_id, call_site, (unsigned long) ptr,
> bytes_req, bytes_alloc, (unsigned long) gfp_flags, node);
> }
>
--
~Randy
On Tue, Aug 19, 2008 at 10:51:32AM -0700, Randy.Dunlap wrote:
> On Tue, 19 Aug 2008, Eduard - Gabriel Munteanu wrote:
>
> > This reverts commit 79cf3d5e207243eecb1c4331c569e17700fa08fa.
> >
> > The reverted commit, while it fixed printk format warnings, it resulted in
> > marker-probe format mismatches. Another approach should be used to fix
> > these warnings.
>
> Such as what?
>
> Can marker probes be fixed instead?
>
> After seeing & fixing lots of various warnings in the last few days,
> I'm thinking that people don't look at/heed warnings nowadays. Sad.
> Maybe there are just so many that they are lost in the noise.
Hi,
Check the next patch in the series, it provides the alternate fix.
I favor this approach more because it involves fewer changes and we
don't have to use things like "%zu" (which make data type size less
apparent).
Cheers,
Eduard
Eduard - Gabriel Munteanu wrote:
> void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
> {
> - return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
> + return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
> }
Could you get rid of the casts by changing the type of parameter of slab_alloc()?
* Eduard - Gabriel Munteanu ([email protected]) wrote:
> On Tue, Aug 19, 2008 at 10:51:32AM -0700, Randy.Dunlap wrote:
> > On Tue, 19 Aug 2008, Eduard - Gabriel Munteanu wrote:
> >
> > > This reverts commit 79cf3d5e207243eecb1c4331c569e17700fa08fa.
> > >
> > > The reverted commit, while it fixed printk format warnings, it resulted in
> > > marker-probe format mismatches. Another approach should be used to fix
> > > these warnings.
> >
> > Such as what?
> >
> > Can marker probes be fixed instead?
> >
> > After seeing & fixing lots of various warnings in the last few days,
> > I'm thinking that people don't look at/heed warnings nowadays. Sad.
> > Maybe there are just so many that they are lost in the noise.
>
> Hi,
>
> Check the next patch in the series, it provides the alternate fix.
> I favor this approach more because it involves fewer changes and we
> don't have to use things like "%zu" (which make data type size less
> apparent).
>
Question :
Is this kmemtrace marker meant to be exposed to userspace ?
I suspect not. In all case, not directly. I expect in-kernel probes to
be connected on these markers which will get the arguments they need,
and maybe access the inner data structures. Anyhow, tracepoints should
be used for that, not markers. You can later put markers in the probes
which are themselves connected to tracepoints.
Tracepoints = in-kernel tracing API.
Markers = Data-formatting tracing API, meant to export the data either
to user-space in text or binary format.
See
http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=shortlog
tracepoint-related patches.
Mathieu
>
> Cheers,
> Eduard
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, Aug 19, 2008 at 01:14:01PM -0500, Christoph Lameter wrote:
> Eduard - Gabriel Munteanu wrote:
>
> > void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
> > {
> > - return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
> > + return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
> > }
>
> Could you get rid of the casts by changing the type of parameter of slab_alloc()?
I just looked at it and it isn't a trivial change. slab_alloc() calls
other functions which expect a void ptr. Even if slab_alloc() were to
take an unsigned long and then cast it to a void ptr, other functions do
call slab_alloc() with void ptr arguments (so the casts would move
there).
I'd rather have this merged as it is and change things later, so that
kmemtrace gets some testing from Pekka and others.
On Tue, Aug 19, 2008 at 02:16:53PM -0400, Mathieu Desnoyers wrote:
> Question :
>
> Is this kmemtrace marker meant to be exposed to userspace ?
>
> I suspect not. In all case, not directly. I expect in-kernel probes to
> be connected on these markers which will get the arguments they need,
> and maybe access the inner data structures. Anyhow, tracepoints should
> be used for that, not markers. You can later put markers in the probes
> which are themselves connected to tracepoints.
>
> Tracepoints = in-kernel tracing API.
>
> Markers = Data-formatting tracing API, meant to export the data either
> to user-space in text or binary format.
>
> See
>
> http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=shortlog
>
> tracepoint-related patches.
I think we're ready to try tracepoints. Pekka, could you merge Mathieu's
tracepoints or otherwise provide a branch where I could submit a
tracepoint conversion patch for kmemtrace?
> Mathieu
>
Eduard - Gabriel Munteanu wrote:
> > void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
> > {
> > - return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
> > + return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
> > }
On Tue, 19 Aug 2008, Christoph Lameter wrote:
> Could you get rid of the casts by changing the type of parameter of slab_alloc()?
Yeah. How about something like this?
Pekka
>From f4d0f88f5b460afb2fd5dff75d4fcc0033e85a48 Mon Sep 17 00:00:00 2001
From: Eduard - Gabriel Munteanu <[email protected]>
Date: Tue, 19 Aug 2008 20:43:25 +0300
Subject: [PATCH] SLUB: Replace __builtin_return_address(0) with _RET_IP_.
This patch replaces __builtin_return_address(0) with _RET_IP_, since a
previous patch moved _RET_IP_ and _THIS_IP_ to include/linux/kernel.h and
they're widely available now. This makes for shorter and easier to read
code.
[[email protected]: remove _RET_IP_ casts to void pointer]
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
---
include/linux/slab.h | 4 ++--
mm/slab.c | 8 ++++----
mm/slub.c | 44 ++++++++++++++++++++++----------------------
3 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 5ff9676..5ebb8df 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -225,7 +225,7 @@ static inline void *kmem_cache_alloc_node(struct kmem_cache *cachep,
* request comes from.
*/
#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB)
-extern void *__kmalloc_track_caller(size_t, gfp_t, void*);
+extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long);
#define kmalloc_track_caller(size, flags) \
__kmalloc_track_caller(size, flags, __builtin_return_address(0))
#else
@@ -243,7 +243,7 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, void*);
* allocation request comes from.
*/
#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_SLUB)
-extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, void *);
+extern void *__kmalloc_node_track_caller(size_t, gfp_t, int, unsigned long);
#define kmalloc_node_track_caller(size, flags, node) \
__kmalloc_node_track_caller(size, flags, node, \
__builtin_return_address(0))
diff --git a/mm/slab.c b/mm/slab.c
index e76eee4..6a0e633 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3685,9 +3685,9 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
EXPORT_SYMBOL(__kmalloc_node);
void *__kmalloc_node_track_caller(size_t size, gfp_t flags,
- int node, void *caller)
+ int node, unsigned long caller)
{
- return __do_kmalloc_node(size, flags, node, caller);
+ return __do_kmalloc_node(size, flags, node, (void *)caller);
}
EXPORT_SYMBOL(__kmalloc_node_track_caller);
#else
@@ -3729,9 +3729,9 @@ void *__kmalloc(size_t size, gfp_t flags)
}
EXPORT_SYMBOL(__kmalloc);
-void *__kmalloc_track_caller(size_t size, gfp_t flags, void *caller)
+void *__kmalloc_track_caller(size_t size, gfp_t flags, unsigned long caller)
{
- return __do_kmalloc(size, flags, caller);
+ return __do_kmalloc(size, flags, (void *)caller);
}
EXPORT_SYMBOL(__kmalloc_track_caller);
diff --git a/mm/slub.c b/mm/slub.c
index 4f5b961..804ca82 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -177,7 +177,7 @@ static LIST_HEAD(slab_caches);
* Tracking user of a slab.
*/
struct track {
- void *addr; /* Called from address */
+ unsigned long addr; /* Called from address */
int cpu; /* Was running on cpu */
int pid; /* Pid context */
unsigned long when; /* When did the operation occur */
@@ -366,7 +366,7 @@ static struct track *get_track(struct kmem_cache *s, void *object,
}
static void set_track(struct kmem_cache *s, void *object,
- enum track_item alloc, void *addr)
+ enum track_item alloc, unsigned long addr)
{
struct track *p;
@@ -390,8 +390,8 @@ static void init_tracking(struct kmem_cache *s, void *object)
if (!(s->flags & SLAB_STORE_USER))
return;
- set_track(s, object, TRACK_FREE, NULL);
- set_track(s, object, TRACK_ALLOC, NULL);
+ set_track(s, object, TRACK_FREE, 0);
+ set_track(s, object, TRACK_ALLOC, 0);
}
static void print_track(const char *s, struct track *t)
@@ -400,7 +400,7 @@ static void print_track(const char *s, struct track *t)
return;
printk(KERN_ERR "INFO: %s in %pS age=%lu cpu=%u pid=%d\n",
- s, t->addr, jiffies - t->when, t->cpu, t->pid);
+ s, (void *)t->addr, jiffies - t->when, t->cpu, t->pid);
}
static void print_tracking(struct kmem_cache *s, void *object)
@@ -865,7 +865,7 @@ static void setup_object_debug(struct kmem_cache *s, struct page *page,
}
static int alloc_debug_processing(struct kmem_cache *s, struct page *page,
- void *object, void *addr)
+ void *object, unsigned long addr)
{
if (!check_slab(s, page))
goto bad;
@@ -905,7 +905,7 @@ bad:
}
static int free_debug_processing(struct kmem_cache *s, struct page *page,
- void *object, void *addr)
+ void *object, unsigned long addr)
{
if (!check_slab(s, page))
goto fail;
@@ -1498,8 +1498,8 @@ static inline int node_match(struct kmem_cache_cpu *c, int node)
* we need to allocate a new slab. This is the slowest path since it involves
* a call to the page allocator and the setup of a new slab.
*/
-static void *__slab_alloc(struct kmem_cache *s,
- gfp_t gfpflags, int node, void *addr, struct kmem_cache_cpu *c)
+static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
+ unsigned long addr, struct kmem_cache_cpu *c)
{
void **object;
struct page *new;
@@ -1583,7 +1583,7 @@ debug:
* Otherwise we can simply pick the next object from the lockless free list.
*/
static __always_inline void *slab_alloc(struct kmem_cache *s,
- gfp_t gfpflags, int node, void *addr)
+ gfp_t gfpflags, int node, unsigned long addr)
{
void **object;
struct kmem_cache_cpu *c;
@@ -1612,14 +1612,14 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
{
- return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
+ return slab_alloc(s, gfpflags, -1, _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_alloc);
#ifdef CONFIG_NUMA
void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
{
- return slab_alloc(s, gfpflags, node, __builtin_return_address(0));
+ return slab_alloc(s, gfpflags, node, _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_alloc_node);
#endif
@@ -1633,7 +1633,7 @@ EXPORT_SYMBOL(kmem_cache_alloc_node);
* handling required then we can return immediately.
*/
static void __slab_free(struct kmem_cache *s, struct page *page,
- void *x, void *addr, unsigned int offset)
+ void *x, unsigned long addr, unsigned int offset)
{
void *prior;
void **object = (void *)x;
@@ -1703,7 +1703,7 @@ debug:
* with all sorts of special processing.
*/
static __always_inline void slab_free(struct kmem_cache *s,
- struct page *page, void *x, void *addr)
+ struct page *page, void *x, unsigned long addr)
{
void **object = (void *)x;
struct kmem_cache_cpu *c;
@@ -1730,7 +1730,7 @@ void kmem_cache_free(struct kmem_cache *s, void *x)
page = virt_to_head_page(x);
- slab_free(s, page, x, __builtin_return_address(0));
+ slab_free(s, page, x, _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -2657,7 +2657,7 @@ void *__kmalloc(size_t size, gfp_t flags)
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, -1, __builtin_return_address(0));
+ return slab_alloc(s, flags, -1, _RET_IP_);
}
EXPORT_SYMBOL(__kmalloc);
@@ -2685,7 +2685,7 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, node, __builtin_return_address(0));
+ return slab_alloc(s, flags, node, _RET_IP_);
}
EXPORT_SYMBOL(__kmalloc_node);
#endif
@@ -2742,7 +2742,7 @@ void kfree(const void *x)
put_page(page);
return;
}
- slab_free(page->slab, page, object, __builtin_return_address(0));
+ slab_free(page->slab, page, object, _RET_IP_);
}
EXPORT_SYMBOL(kfree);
@@ -3198,7 +3198,7 @@ static struct notifier_block __cpuinitdata slab_notifier = {
#endif
-void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, void *caller)
+void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, unsigned long caller)
{
struct kmem_cache *s;
@@ -3214,7 +3214,7 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, void *caller)
}
void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
- int node, void *caller)
+ int node, unsigned long caller)
{
struct kmem_cache *s;
@@ -3425,7 +3425,7 @@ static void resiliency_test(void) {};
struct location {
unsigned long count;
- void *addr;
+ unsigned long addr;
long long sum_time;
long min_time;
long max_time;
@@ -3473,7 +3473,7 @@ static int add_location(struct loc_track *t, struct kmem_cache *s,
{
long start, end, pos;
struct location *l;
- void *caddr;
+ unsigned long caddr;
unsigned long age = jiffies - track->when;
start = -1;
--
1.5.4.3
Mathieu Desnoyers <[email protected]> writes:
> [...]
> Is this kmemtrace marker meant to be exposed to userspace ?
> I suspect not.
> [...]
> Tracepoints = in-kernel tracing API.
> Markers = Data-formatting tracing API, meant to export the data either
> to user-space in text or binary format.
FWIW, that was certainly not the intent of markers. It was to try to
satisfy both sorts of uses with relative type-safety and a minimum of
code. Tracepoints may be nice if one needs somewhat (how much?) more
performance, and is willing to burden someone else with the necessary
extra code (such as tracepoint-to-marker conversion modules) to expose
the same data to "userspace" tools like lttng/systemtap.
- FChE
Eduard - Gabriel Munteanu wrote:
> On Tue, Aug 19, 2008 at 01:14:01PM -0500, Christoph Lameter wrote:
>> Eduard - Gabriel Munteanu wrote:
>>
>>> void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
>>> {
>>> - return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
>>> + return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
>>> }
>> Could you get rid of the casts by changing the type of parameter of slab_alloc()?
>
> I just looked at it and it isn't a trivial change. slab_alloc() calls
> other functions which expect a void ptr. Even if slab_alloc() were to
> take an unsigned long and then cast it to a void ptr, other functions do
> call slab_alloc() with void ptr arguments (so the casts would move
> there).
>
> I'd rather have this merged as it is and change things later, so that
> kmemtrace gets some testing from Pekka and others.
>
Well maybe this patch will do it then:
Subject: slub: Use _RET_IP and use "unsigned long" for kernel text addresses
Use _RET_IP_ instead of buildint_return_address() and make slub use unsigned long
instead of void * for addresses.
Signed-off-by: Christoph Lameter <[email protected]>
---
mm/slub.c | 46 +++++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
Index: linux-2.6/mm/slub.c
===================================================================
--- linux-2.6.orig/mm/slub.c 2008-08-19 11:40:52.332357770 -0700
+++ linux-2.6/mm/slub.c 2008-08-19 11:52:17.479064425 -0700
@@ -177,7 +177,7 @@ static LIST_HEAD(slab_caches);
* Tracking user of a slab.
*/
struct track {
- void *addr; /* Called from address */
+ unsigned long addr; /* Called from address */
int cpu; /* Was running on cpu */
int pid; /* Pid context */
unsigned long when; /* When did the operation occur */
@@ -366,7 +366,7 @@ static struct track *get_track(struct km
}
static void set_track(struct kmem_cache *s, void *object,
- enum track_item alloc, void *addr)
+ enum track_item alloc, unsigned long addr)
{
struct track *p;
@@ -390,8 +390,8 @@ static void init_tracking(struct kmem_ca
if (!(s->flags & SLAB_STORE_USER))
return;
- set_track(s, object, TRACK_FREE, NULL);
- set_track(s, object, TRACK_ALLOC, NULL);
+ set_track(s, object, TRACK_FREE, 0L);
+ set_track(s, object, TRACK_ALLOC, 0L);
}
static void print_track(const char *s, struct track *t)
@@ -399,7 +399,7 @@ static void print_track(const char *s, s
if (!t->addr)
return;
- printk(KERN_ERR "INFO: %s in %pS age=%lu cpu=%u pid=%d\n",
+ printk(KERN_ERR "INFO: %s in %lxS age=%lu cpu=%u pid=%d\n",
s, t->addr, jiffies - t->when, t->cpu, t->pid);
}
@@ -865,7 +865,7 @@ static void setup_object_debug(struct km
}
static int alloc_debug_processing(struct kmem_cache *s, struct page *page,
- void *object, void *addr)
+ void *object, unsigned long addr)
{
if (!check_slab(s, page))
goto bad;
@@ -905,7 +905,7 @@ bad:
}
static int free_debug_processing(struct kmem_cache *s, struct page *page,
- void *object, void *addr)
+ void *object, unsigned long addr)
{
if (!check_slab(s, page))
goto fail;
@@ -1028,10 +1028,10 @@ static inline void setup_object_debug(st
struct page *page, void *object) {}
static inline int alloc_debug_processing(struct kmem_cache *s,
- struct page *page, void *object, void *addr) { return 0; }
+ struct page *page, void *object, unsigned long addr) { return 0; }
static inline int free_debug_processing(struct kmem_cache *s,
- struct page *page, void *object, void *addr) { return 0; }
+ struct page *page, void *object, unsigned long) { return 0; }
static inline int slab_pad_check(struct kmem_cache *s, struct page *page)
{ return 1; }
@@ -1499,7 +1499,7 @@ static inline int node_match(struct kmem
* a call to the page allocator and the setup of a new slab.
*/
static void *__slab_alloc(struct kmem_cache *s,
- gfp_t gfpflags, int node, void *addr, struct kmem_cache_cpu *c)
+ gfp_t gfpflags, int node, unsigned long addr, struct kmem_cache_cpu *c)
{
void **object;
struct page *new;
@@ -1583,7 +1583,7 @@ debug:
* Otherwise we can simply pick the next object from the lockless free list.
*/
static __always_inline void *slab_alloc(struct kmem_cache *s,
- gfp_t gfpflags, int node, void *addr)
+ gfp_t gfpflags, int node, unsigned long addr)
{
void **object;
struct kmem_cache_cpu *c;
@@ -1612,14 +1612,14 @@ static __always_inline void *slab_alloc(
void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
{
- return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
+ return slab_alloc(s, gfpflags, -1, _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_alloc);
#ifdef CONFIG_NUMA
void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t gfpflags, int node)
{
- return slab_alloc(s, gfpflags, node, __builtin_return_address(0));
+ return slab_alloc(s, gfpflags, node, _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_alloc_node);
#endif
@@ -1633,7 +1633,7 @@ EXPORT_SYMBOL(kmem_cache_alloc_node);
* handling required then we can return immediately.
*/
static void __slab_free(struct kmem_cache *s, struct page *page,
- void *x, void *addr, unsigned int offset)
+ void *x, unsigned long addr, unsigned int offset)
{
void *prior;
void **object = (void *)x;
@@ -1703,7 +1703,7 @@ debug:
* with all sorts of special processing.
*/
static __always_inline void slab_free(struct kmem_cache *s,
- struct page *page, void *x, void *addr)
+ struct page *page, void *x, unsigned long addr)
{
void **object = (void *)x;
struct kmem_cache_cpu *c;
@@ -1730,7 +1730,7 @@ void kmem_cache_free(struct kmem_cache *
page = virt_to_head_page(x);
- slab_free(s, page, x, __builtin_return_address(0));
+ slab_free(s, page, x, _RET_IP_);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -2657,7 +2657,7 @@ void *__kmalloc(size_t size, gfp_t flags
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, -1, __builtin_return_address(0));
+ return slab_alloc(s, flags, -1, _RET_IP_);
}
EXPORT_SYMBOL(__kmalloc);
@@ -2685,7 +2685,7 @@ void *__kmalloc_node(size_t size, gfp_t
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, node, __builtin_return_address(0));
+ return slab_alloc(s, flags, node, _RET_IP_);
}
EXPORT_SYMBOL(__kmalloc_node);
#endif
@@ -2742,7 +2742,7 @@ void kfree(const void *x)
put_page(page);
return;
}
- slab_free(page->slab, page, object, __builtin_return_address(0));
+ slab_free(page->slab, page, object, _RET_IP_);
}
EXPORT_SYMBOL(kfree);
@@ -3210,7 +3210,7 @@ void *__kmalloc_track_caller(size_t size
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, gfpflags, -1, caller);
+ return slab_alloc(s, gfpflags, -1, (unsigned long)caller);
}
void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
@@ -3226,7 +3226,7 @@ void *__kmalloc_node_track_caller(size_t
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, gfpflags, node, caller);
+ return slab_alloc(s, gfpflags, node, (unsigned long)caller);
}
#ifdef CONFIG_SLUB_DEBUG
@@ -3425,7 +3425,7 @@ static void resiliency_test(void) {};
struct location {
unsigned long count;
- void *addr;
+ unsigned long addr;
long long sum_time;
long min_time;
long max_time;
@@ -3473,7 +3473,7 @@ static int add_location(struct loc_track
{
long start, end, pos;
struct location *l;
- void *caddr;
+ unsigned long caddr;
unsigned long age = jiffies - track->when;
start = -1;
Christoph Lameter wrote:
> Eduard - Gabriel Munteanu wrote:
>> On Tue, Aug 19, 2008 at 01:14:01PM -0500, Christoph Lameter wrote:
>>> Eduard - Gabriel Munteanu wrote:
>>>
>>>> void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
>>>> {
>>>> - return slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
>>>> + return slab_alloc(s, gfpflags, -1, (void *) _RET_IP_);
>>>> }
>>> Could you get rid of the casts by changing the type of parameter of slab_alloc()?
>> I just looked at it and it isn't a trivial change. slab_alloc() calls
>> other functions which expect a void ptr. Even if slab_alloc() were to
>> take an unsigned long and then cast it to a void ptr, other functions do
>> call slab_alloc() with void ptr arguments (so the casts would move
>> there).
>>
>> I'd rather have this merged as it is and change things later, so that
>> kmemtrace gets some testing from Pekka and others.
>>
>
> Well maybe this patch will do it then:
>
> Subject: slub: Use _RET_IP and use "unsigned long" for kernel text addresses
>
> Use _RET_IP_ instead of buildint_return_address() and make slub use unsigned long
> instead of void * for addresses.
>
> Signed-off-by: Christoph Lameter <[email protected]>
Heh, heh. I'm happy to take your patch or alternatively you can ACK mine
(which is slightly different):
http://lkml.org/lkml/2008/8/19/336
On Tue, Aug 19, 2008 at 01:56:41PM -0500, Christoph Lameter wrote:
> Well maybe this patch will do it then:
>
> Subject: slub: Use _RET_IP and use "unsigned long" for kernel text addresses
>
> Use _RET_IP_ instead of buildint_return_address() and make slub use unsigned long
> instead of void * for addresses.
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> ---
> mm/slub.c | 46 +++++++++++++++++++++++-----------------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
It seems Pekka just submitted something like this. Though I think using 0L
should be replaced with 0UL to be fully correct.
Pekka, should I test one of these variants and resubmit, or will you
merge it by yourself?
Christoph Lameter wrote:
> static void print_track(const char *s, struct track *t)
> @@ -399,7 +399,7 @@ static void print_track(const char *s, s
> if (!t->addr)
> return;
>
> - printk(KERN_ERR "INFO: %s in %pS age=%lu cpu=%u pid=%d\n",
> + printk(KERN_ERR "INFO: %s in %lxS age=%lu cpu=%u pid=%d\n",
> s, t->addr, jiffies - t->when, t->cpu, t->pid);
This looks wrong. The '%pS' thingy has a special purpose:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7daf705f362e349983e92037a198b8821db198af
Eduard - Gabriel Munteanu wrote:
> On Tue, Aug 19, 2008 at 01:56:41PM -0500, Christoph Lameter wrote:
>
>> Well maybe this patch will do it then:
>>
>> Subject: slub: Use _RET_IP and use "unsigned long" for kernel text addresses
>>
>> Use _RET_IP_ instead of buildint_return_address() and make slub use unsigned long
>> instead of void * for addresses.
>>
>> Signed-off-by: Christoph Lameter <[email protected]>
>>
>> ---
>> mm/slub.c | 46 +++++++++++++++++++++++-----------------------
>> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> It seems Pekka just submitted something like this. Though I think using 0L
> should be replaced with 0UL to be fully correct.
Fixed, thanks!
> Pekka, should I test one of these variants and resubmit, or will you
> merge it by yourself?
I'm merging my patch to the kmemtrace branch now.
Eduard - Gabriel Munteanu wrote:
> Corrected the ABI description and the kmemtrace usage guide. Thanks to
> Randy Dunlap for noticing these errors.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
Applied, thanks!
Eduard - Gabriel Munteanu wrote:
> This adds hooks for the SLUB allocator, to allow tracing with kmemtrace.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
Applied, thanks!
Eduard - Gabriel Munteanu wrote:
> Fix the problem "kmemtrace: fix printk format warnings" attempted to fix,
> but resulted in marker-probe format mismatch warnings. Instead of carrying
> size_t into probes, we get rid of it by casting to unsigned long, just as
> we did with gfp_t.
>
> This way, we don't need to change marker format strings and we don't have
> to rely on other format specifiers like "%zu", making for consistent use
> of more generic data types (since there are no format specifiers for
> gfp_t, for example).
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
Applied, thanks!
Eduard - Gabriel Munteanu wrote:
> On Tue, Aug 19, 2008 at 02:16:53PM -0400, Mathieu Desnoyers wrote:
>> Question :
>>
>> Is this kmemtrace marker meant to be exposed to userspace ?
>>
>> I suspect not. In all case, not directly. I expect in-kernel probes to
>> be connected on these markers which will get the arguments they need,
>> and maybe access the inner data structures. Anyhow, tracepoints should
>> be used for that, not markers. You can later put markers in the probes
>> which are themselves connected to tracepoints.
>>
>> Tracepoints = in-kernel tracing API.
>>
>> Markers = Data-formatting tracing API, meant to export the data either
>> to user-space in text or binary format.
>>
>> See
>>
>> http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=shortlog
>>
>> tracepoint-related patches.
>
> I think we're ready to try tracepoints. Pekka, could you merge Mathieu's
> tracepoints or otherwise provide a branch where I could submit a
> tracepoint conversion patch for kmemtrace?
Sorry, that's too much of a hassle for me. I'll happily take your
conversion patch once tracepoints hit mainline. Mathieu, are you aiming
for 2.6.28?
Eduard - Gabriel Munteanu wrote:
> This reverts commit 79cf3d5e207243eecb1c4331c569e17700fa08fa.
>
> The reverted commit, while it fixed printk format warnings, it resulted in
> marker-probe format mismatches. Another approach should be used to fix
> these warnings.
I simply dropped Randy's patch so the revert wasn't needed.
On Tue, 19 Aug 2008, Eduard - Gabriel Munteanu wrote:
> On Tue, Aug 19, 2008 at 10:51:32AM -0700, Randy.Dunlap wrote:
> > On Tue, 19 Aug 2008, Eduard - Gabriel Munteanu wrote:
> >
> > > This reverts commit 79cf3d5e207243eecb1c4331c569e17700fa08fa.
> > >
> > > The reverted commit, while it fixed printk format warnings, it resulted in
> > > marker-probe format mismatches. Another approach should be used to fix
> > > these warnings.
> >
> > Such as what?
> >
> > Can marker probes be fixed instead?
Did you answer this?
> > After seeing & fixing lots of various warnings in the last few days,
> > I'm thinking that people don't look at/heed warnings nowadays. Sad.
> > Maybe there are just so many that they are lost in the noise.
>
> Hi,
>
> Check the next patch in the series, it provides the alternate fix.
Yep, I saw that later.
> I favor this approach more because it involves fewer changes and we
> don't have to use things like "%zu" (which make data type size less
> apparent).
%zu is regular C language. I.e., I don't get the data type not being
apparent issue...
Maybe kmemtrace should just make everything be long long... :(
--
~Randy
Pekka Enberg wrote:
> This looks wrong. The '%pS' thingy has a special purpose:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7daf705f362e349983e92037a198b8821db198af
True. Just had 10 minutes to do the patch. Can someone stitch all of the good
things from all three patches together?
Christoph Lameter wrote:
> Pekka Enberg wrote:
>
>> This looks wrong. The '%pS' thingy has a special purpose:
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7daf705f362e349983e92037a198b8821db198af
>
> True. Just had 10 minutes to do the patch. Can someone stitch all of the good
> things from all three patches together?
I already did that:
http://git.kernel.org/?p=linux/kernel/git/penberg/slab-2.6.git;a=commitdiff;h=3c0a0d0e24234704387f6356ffc7b47758dc1e05
It's compiled-tested too. I had to do some changes to
include/linux/slab.h as well.
Pekka
* Pekka Enberg ([email protected]) wrote:
> Eduard - Gabriel Munteanu wrote:
>> On Tue, Aug 19, 2008 at 02:16:53PM -0400, Mathieu Desnoyers wrote:
>>> Question :
>>>
>>> Is this kmemtrace marker meant to be exposed to userspace ?
>>>
>>> I suspect not. In all case, not directly. I expect in-kernel probes to
>>> be connected on these markers which will get the arguments they need,
>>> and maybe access the inner data structures. Anyhow, tracepoints should
>>> be used for that, not markers. You can later put markers in the probes
>>> which are themselves connected to tracepoints.
>>>
>>> Tracepoints = in-kernel tracing API.
>>>
>>> Markers = Data-formatting tracing API, meant to export the data either
>>> to user-space in text or binary format.
>>>
>>> See
>>>
>>> http://git.kernel.org/?p=linux/kernel/git/compudj/linux-2.6-lttng.git;a=shortlog
>>>
>>> tracepoint-related patches.
>> I think we're ready to try tracepoints. Pekka, could you merge Mathieu's
>> tracepoints or otherwise provide a branch where I could submit a
>> tracepoint conversion patch for kmemtrace?
>
> Sorry, that's too much of a hassle for me. I'll happily take your
> conversion patch once tracepoints hit mainline. Mathieu, are you aiming for
> 2.6.28?
Probably. it's in -tip right now, and the new ftrace depends on it. So
at least 2.6.28 yes.
Mathieu
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Pekka Enberg wrote:
> It's compiled-tested too. I had to do some changes to
> include/linux/slab.h as well.
Please recompile with CONFIG_SLUB_DEBUG off to find some breakage. F.e.
static inline int alloc_debug_processing(struct kmem_cache *s,
struct page *page, void *object, void *addr) { return 0; }
static inline int free_debug_processing(struct kmem_cache *s,
struct page *page, void *object, void *addr) { return 0; }
Christoph Lameter wrote:
> Pekka Enberg wrote:
>
>> It's compiled-tested too. I had to do some changes to
>> include/linux/slab.h as well.
>
> Please recompile with CONFIG_SLUB_DEBUG off to find some breakage. F.e.
>
> static inline int alloc_debug_processing(struct kmem_cache *s,
> struct page *page, void *object, void *addr) { return 0; }
>
> static inline int free_debug_processing(struct kmem_cache *s,
> struct page *page, void *object, void *addr) { return 0; }
Fixed, thanks.
http://git.kernel.org/?p=linux/kernel/git/penberg/slab-2.6.git;a=commitdiff;h=04be8567dab2a793f4ec8050c65af6845e6e3af3
On Tue, Aug 19, 2008 at 12:32:14PM -0700, Randy.Dunlap wrote:
> > >
> > > Such as what?
> > >
> > > Can marker probes be fixed instead?
>
> Did you answer this?
Yes, they can be fixed, but the probe functions will likely show
warnings unless the way we parse vargs is fixed as well.
> > > After seeing & fixing lots of various warnings in the last few days,
> > > I'm thinking that people don't look at/heed warnings nowadays. Sad.
> > > Maybe there are just so many that they are lost in the noise.
> >
> > Hi,
> >
> > Check the next patch in the series, it provides the alternate fix.
>
> Yep, I saw that later.
>
> > I favor this approach more because it involves fewer changes and we
> > don't have to use things like "%zu" (which make data type size less
> > apparent).
>
> %zu is regular C language. I.e., I don't get the data type not being
> apparent issue...
Yes, I know. But I feel like using unsigned long is consistent with the
way we handle the call site pointers and gfp_t. Pointers are cast to
unsigned long (in _RET_IP_) and size_t's actual range and size is more
apparent if it's cast to unsigned long as well (since allocation sizes
should scale the same as pointers do, and we know sizeof(unsigned long)
== sizeof(void *) on GCC).
> Maybe kmemtrace should just make everything be long long... :(
I was merely trying to sort this out faster and more consistent.
> --
> ~Randy