Hello everybody,
I hopefully fixed previous complaints. Also wrote some documentation and
fixed some missing stuff in SLAB.
Please take a look and comment.
BTW, see Documentation/vm/kmemtrace.txt for details on how to use this and
for info on design details.
Eduard
Eduard - Gabriel Munteanu (4):
kmemtrace: Core implementation.
kmemtrace: SLAB hooks.
kmemtrace: SLUB hooks.
kmemtrace: SLOB hooks.
Documentation/kernel-parameters.txt | 6 +
Documentation/vm/kmemtrace.txt | 96 ++++++++++++++++
MAINTAINERS | 6 +
include/linux/kmemtrace.h | 110 ++++++++++++++++++
include/linux/slab_def.h | 56 ++++++++-
include/linux/slub_def.h | 9 ++-
init/main.c | 2 +
lib/Kconfig.debug | 4 +
mm/Makefile | 2 +-
mm/kmemtrace.c | 208 +++++++++++++++++++++++++++++++++++
mm/slab.c | 61 +++++++++-
mm/slob.c | 37 +++++-
mm/slub.c | 47 +++++++-
13 files changed, 617 insertions(+), 27 deletions(-)
create mode 100644 Documentation/vm/kmemtrace.txt
create mode 100644 include/linux/kmemtrace.h
create mode 100644 mm/kmemtrace.c
kmemtrace provides tracing for slab allocator functions, such as kmalloc,
kfree, kmem_cache_alloc, kmem_cache_free etc.. Collected data is then fed
to the userspace application in order to analyse allocation hotspots,
internal fragmentation and so on, making it possible to see how well an
allocator performs, as well as debug and profile kernel code.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
Documentation/kernel-parameters.txt | 6 +
Documentation/vm/kmemtrace.txt | 96 ++++++++++++++++
MAINTAINERS | 6 +
include/linux/kmemtrace.h | 110 ++++++++++++++++++
init/main.c | 2 +
lib/Kconfig.debug | 4 +
mm/Makefile | 2 +-
mm/kmemtrace.c | 208 +++++++++++++++++++++++++++++++++++
8 files changed, 433 insertions(+), 1 deletions(-)
create mode 100644 Documentation/vm/kmemtrace.txt
create mode 100644 include/linux/kmemtrace.h
create mode 100644 mm/kmemtrace.c
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index b52f47d..b230aff 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -49,6 +49,7 @@ parameter is applicable:
ISAPNP ISA PnP code is enabled.
ISDN Appropriate ISDN support is enabled.
JOY Appropriate joystick support is enabled.
+ KMEMTRACE kmemtrace is enabled.
LIBATA Libata driver is enabled
LP Printer support is enabled.
LOOP Loopback device support is enabled.
@@ -941,6 +942,11 @@ and is between 256 and 4096 characters. It is defined in the file
use the HighMem zone if it exists, and the Normal
zone if it does not.
+ kmemtrace.subbufs=n [KNL,KMEMTRACE] Overrides the number of
+ subbufs kmemtrace's relay channel has. Set this
+ higher than default (KMEMTRACE_N_SUBBUFS in code) if
+ you experience buffer overruns.
+
movablecore=nn[KMG] [KNL,X86-32,IA-64,PPC,X86-64] This parameter
is similar to kernelcore except it specifies the
amount of memory used for migratable allocations.
diff --git a/Documentation/vm/kmemtrace.txt b/Documentation/vm/kmemtrace.txt
new file mode 100644
index 0000000..1147ecb
--- /dev/null
+++ b/Documentation/vm/kmemtrace.txt
@@ -0,0 +1,96 @@
+ kmemtrace - Kernel Memory Tracer
+
+ by Eduard - Gabriel Munteanu
+ <[email protected]>
+
+
+I. Design and goals
+===================
+
+kmemtrace was designed to handle rather large amounts of data. Thus, it uses
+the relay interface to export whatever is logged to userspace, which then
+stores it. Analysis and reporting is done asynchronously, that is, after the
+data is collected and stored. By design, it allows one to log and analyse
+on different machines and different arches.
+
+As this is a debugging feature, kmemtrace's ABI is not designed to be very
+stable, although this may happen in the future if it's deemed mature and
+sufficient. So the userspace tool does not contain a copy of the kernel
+header. Instead, the ABI allows checking if the logged data matches the
+userspace tool. Well, what I said about ABI stability isn't totally true:
+while I've tried hard to cover all possible (and useful) use cases, I don't
+want it frozen in the current state. I anticipate the ABI will be _quite_
+stable, even across multiple stable kernel versions, but I don't make any
+guarantees regarding this matter.
+
+Summary of design goals:
+ - allow logging and analysis to be done across different machines
+ - be fast and anticipate usage in high-load environments (*)
+ - be reasonably extensible
+ - have a _reasonably_ (not completely) stable ABI
+
+(*) - one of the reasons Pekka Enberg's original userspace data analysis
+ tool's code was rewritten from Perl to C (although this is more than a
+ simple conversion)
+
+
+II. Quick usage guide
+=====================
+
+1) Get a kernel that supports kmemtrace and build it accordingly (i.e. enable
+CONFIG_KMEMTRACE).
+
+2) Get the userspace tool and build it:
+$ git-clone git://repo.or.cz/kmemtrace-user.git # current repository
+$ cd kmemtrace-user/
+$ autoreconf
+$ ./configure # Supply KERNEL_SOURCES=/path/to/sources/ if you're
+ # _not_ running this on a kmemtrace-enabled kernel.
+$ make
+
+3) Boot the kmemtrace-enabled kernel if you haven't, preferably in the
+'single' runlevel (so that relay buffers don't fill up easily), and run
+kmemtrace:
+# '$' does not mean user, but root here.
+$ mount -t debugfs none /debug
+$ mount -t proc none /proc
+$ cd path/to/kmemtrace-user/
+$ ./kmemtraced
+Wait a bit, then stop it with CTRL+C.
+$ cat /debug/kmemtrace/total_overruns # Check if we didn't overrun, should
+ # be zero.
+$ (Optionally) [Run kmemtrace_check separately on each cpu[0-9]*.out file to
+ check its correctness]
+$ ./kmemtrace-report
+
+Now you should have a nice and short summary of how the allocator performs.
+
+III. FAQ and known issues
+=========================
+Q: 'cat /debug/kmemtrace/total_overruns' is non-zero, how do I fix this?
+Should I worry?
+A: If it's non-zero, this affects kmemtrace's accuracy, depending on how
+large the number is. You can fix it by supplying a higher
+'kmemtrace.subbufs=N' kernel parameter.
+---
+
+Q: kmemtrace_check reports errors, how do I fix this? Should I worry?
+A: This is a bug and should be reported. It can occur for a variety of
+reasons:
+ - possible bugs in relay code
+ - possible misuse of relay by kmemtrace
+ - timestamps being collected unorderly
+Or you may fix it yourself and send us a patch.
+---
+
+Q: kmemtrace_report shows many errors, how do I fix this? Should I worry?
+A: This is a known issue and I'm working on it. These might be true errors
+in kernel code, which may have inconsistent behavior (e.g. allocating memory
+with kmem_cache_alloc() and freeing it with kfree()). Pekka Enberg pointed
+out this behavior may work with SLAB, but may fail with other allocators.
+
+It may also be due to lack of tracing in some unusual allocator functions.
+
+We don't want bug reports regarding this issue yet.
+---
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 56a2f67..e967bc2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2425,6 +2425,12 @@ M: [email protected]
L: [email protected]
S: Maintained
+KMEMTRACE
+P: Eduard - Gabriel Munteanu
+M: [email protected]
+L: [email protected]
+S: Maintained
+
KPROBES
P: Ananth N Mavinakayanahalli
M: [email protected]
diff --git a/include/linux/kmemtrace.h b/include/linux/kmemtrace.h
new file mode 100644
index 0000000..da69d22
--- /dev/null
+++ b/include/linux/kmemtrace.h
@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2008 Eduard - Gabriel Munteanu
+ *
+ * This file is released under GPL version 2.
+ */
+
+#ifndef _LINUX_KMEMTRACE_H
+#define _LINUX_KMEMTRACE_H
+
+#include <linux/types.h>
+
+/* ABI definition starts here. */
+
+#define KMEMTRACE_ABI_VERSION 1
+
+enum kmemtrace_event_id {
+ KMEMTRACE_EVENT_NULL = 0, /* Erroneous event. */
+ KMEMTRACE_EVENT_ALLOC,
+ KMEMTRACE_EVENT_FREE,
+};
+
+enum kmemtrace_type_id {
+ KMEMTRACE_TYPE_KERNEL = 0, /* kmalloc() / kfree(). */
+ KMEMTRACE_TYPE_CACHE, /* kmem_cache_*(). */
+ KMEMTRACE_TYPE_PAGES, /* __get_free_pages() and friends. */
+};
+
+struct kmemtrace_event {
+ __u16 event_id; /* Allocate or free? */
+ __u16 type_id; /* Kind of allocation/free. */
+ __s32 node; /* Target CPU. */
+ __u64 call_site; /* Caller address. */
+ __u64 ptr; /* Pointer to allocation. */
+ __u64 bytes_req; /* Number of bytes requested. */
+ __u64 bytes_alloc; /* Number of bytes allocated. */
+ __u64 gfp_flags; /* Requested flags. */
+ __s64 timestamp; /* When the operation occured in ns. */
+} __attribute__ ((__packed__));
+
+/* End of ABI definition. */
+
+#ifdef __KERNEL__
+
+#include <linux/marker.h>
+
+#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,
+ unsigned long 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,
+ bytes_req, bytes_alloc, 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,
+ unsigned long 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,
+ unsigned long gfp_flags)
+{
+ kmemtrace_mark_alloc_node(type_id, call_site, ptr,
+ bytes_req, bytes_alloc, gfp_flags, -1);
+}
+
+#endif /* __KERNEL__ */
+
+#endif /* _LINUX_KMEMTRACE_H */
+
diff --git a/init/main.c b/init/main.c
index 057f364..c00659c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -66,6 +66,7 @@
#include <asm/setup.h>
#include <asm/sections.h>
#include <asm/cacheflush.h>
+#include <linux/kmemtrace.h>
#ifdef CONFIG_X86_LOCAL_APIC
#include <asm/smp.h>
@@ -641,6 +642,7 @@ asmlinkage void __init start_kernel(void)
enable_debug_pagealloc();
cpu_hotplug_init();
kmem_cache_init();
+ kmemtrace_init();
debug_objects_mem_init();
idr_init_cache();
setup_per_cpu_pageset();
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d2099f4..6bacab5 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -674,6 +674,10 @@ config FIREWIRE_OHCI_REMOTE_DMA
If unsure, say N.
+config KMEMTRACE
+ bool "Kernel memory tracer"
+ depends on RELAY && DEBUG_FS && MARKERS
+
source "samples/Kconfig"
source "lib/Kconfig.kgdb"
diff --git a/mm/Makefile b/mm/Makefile
index 18c143b..d88a3bc 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -33,4 +33,4 @@ obj-$(CONFIG_MIGRATION) += migrate.o
obj-$(CONFIG_SMP) += allocpercpu.o
obj-$(CONFIG_QUICKLIST) += quicklist.o
obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o
-
+obj-$(CONFIG_KMEMTRACE) += kmemtrace.o
diff --git a/mm/kmemtrace.c b/mm/kmemtrace.c
new file mode 100644
index 0000000..9258010
--- /dev/null
+++ b/mm/kmemtrace.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright (C) 2008 Pekka Enberg, Eduard - Gabriel Munteanu
+ *
+ * This file is released under GPL version 2.
+ */
+
+#include <linux/string.h>
+#include <linux/debugfs.h>
+#include <linux/relay.h>
+#include <linux/module.h>
+#include <linux/marker.h>
+#include <linux/gfp.h>
+#include <linux/kmemtrace.h>
+
+#define KMEMTRACE_SUBBUF_SIZE (8192 * sizeof(struct kmemtrace_event))
+#define KMEMTRACE_N_SUBBUFS 20
+
+static struct rchan *kmemtrace_chan;
+static u32 kmemtrace_buf_overruns;
+static unsigned int kmemtrace_n_subbufs;
+
+static inline void kmemtrace_log_event(struct kmemtrace_event *event)
+{
+ relay_write(kmemtrace_chan, event, sizeof(struct kmemtrace_event));
+}
+
+static void kmemtrace_probe_alloc(void *probe_data, void *call_data,
+ const char *format, va_list *args)
+{
+ unsigned long flags;
+ struct kmemtrace_event ev;
+
+ /*
+ * Don't convert this to use structure initializers,
+ * C99 does not guarantee the rvalues evaluation order.
+ */
+ ev.event_id = KMEMTRACE_EVENT_ALLOC;
+ ev.type_id = va_arg(*args, int);
+ ev.call_site = va_arg(*args, unsigned long);
+ ev.ptr = va_arg(*args, unsigned long);
+ /* Don't trace ignored allocations. */
+ if (!ev.ptr)
+ return;
+ ev.bytes_req = va_arg(*args, unsigned long);
+ ev.bytes_alloc = va_arg(*args, unsigned long);
+ /* ev.timestamp set below, to preserve event ordering. */
+ ev.gfp_flags = va_arg(*args, unsigned long);
+ ev.node = va_arg(*args, int);
+
+ local_irq_save(flags);
+ ev.timestamp = ktime_to_ns(ktime_get());
+ kmemtrace_log_event(&ev);
+ local_irq_restore(flags);
+}
+
+static void kmemtrace_probe_free(void *probe_data, void *call_data,
+ const char *format, va_list *args)
+{
+ unsigned long flags;
+ struct kmemtrace_event ev;
+
+ /*
+ * Don't convert this to use structure initializers,
+ * C99 does not guarantee the rvalues evaluation order.
+ */
+ ev.event_id = KMEMTRACE_EVENT_FREE;
+ ev.type_id = va_arg(*args, int);
+ ev.call_site = va_arg(*args, unsigned long);
+ ev.ptr = va_arg(*args, unsigned long);
+ /* Don't trace ignored allocations. */
+ if (!ev.ptr)
+ return;
+ /* ev.timestamp set below, to preserve event ordering. */
+
+ local_irq_save(flags);
+ ev.timestamp = ktime_to_ns(ktime_get());
+ kmemtrace_log_event(&ev);
+ local_irq_restore(flags);
+}
+
+static struct dentry *
+kmemtrace_create_buf_file(const char *filename, struct dentry *parent,
+ int mode, struct rchan_buf *buf, int *is_global)
+{
+ return debugfs_create_file(filename, mode, parent, buf,
+ &relay_file_operations);
+}
+
+static int kmemtrace_remove_buf_file(struct dentry *dentry)
+{
+ debugfs_remove(dentry);
+
+ return 0;
+}
+
+static int kmemtrace_count_overruns(struct rchan_buf *buf,
+ void *subbuf, void *prev_subbuf,
+ size_t prev_padding)
+{
+ if (relay_buf_full(buf)) {
+ kmemtrace_buf_overruns++;
+ return 0;
+ }
+
+ return 1;
+}
+
+static struct rchan_callbacks relay_callbacks = {
+ .create_buf_file = kmemtrace_create_buf_file,
+ .remove_buf_file = kmemtrace_remove_buf_file,
+ .subbuf_start = kmemtrace_count_overruns,
+};
+
+static struct dentry *kmemtrace_dir;
+static struct dentry *kmemtrace_overruns_dentry;
+
+static void kmemtrace_cleanup(void)
+{
+ relay_close(kmemtrace_chan);
+ marker_probe_unregister("kmemtrace_alloc",
+ kmemtrace_probe_alloc, NULL);
+ marker_probe_unregister("kmemtrace_free",
+ kmemtrace_probe_free, NULL);
+ if (kmemtrace_overruns_dentry)
+ debugfs_remove(kmemtrace_overruns_dentry);
+}
+
+static int __init kmemtrace_setup_late(void)
+{
+ if (!kmemtrace_chan)
+ goto failed;
+
+ kmemtrace_dir = debugfs_create_dir("kmemtrace", NULL);
+ if (!kmemtrace_dir)
+ goto cleanup;
+
+ kmemtrace_overruns_dentry =
+ debugfs_create_u32("total_overruns", S_IRUSR,
+ kmemtrace_dir, &kmemtrace_buf_overruns);
+ if (!kmemtrace_overruns_dentry)
+ goto dir_cleanup;
+
+ if (relay_late_setup_files(kmemtrace_chan, "cpu", kmemtrace_dir))
+ goto overrun_cleanup;
+
+ printk(KERN_INFO "kmemtrace: fully up.\n");
+
+ return 0;
+
+overrun_cleanup:
+ debugfs_remove(kmemtrace_overruns_dentry);
+ kmemtrace_overruns_dentry = NULL;
+dir_cleanup:
+ debugfs_remove(kmemtrace_dir);
+cleanup:
+ kmemtrace_cleanup();
+failed:
+ return 1;
+}
+late_initcall(kmemtrace_setup_late);
+
+static int __init kmemtrace_set_subbuf_size(char *str)
+{
+ get_option(&str, &kmemtrace_n_subbufs);
+ return 0;
+}
+early_param("kmemtrace.subbufs", kmemtrace_set_subbuf_size);
+
+void kmemtrace_init(void)
+{
+ int err;
+
+ if (!kmemtrace_n_subbufs)
+ kmemtrace_n_subbufs = KMEMTRACE_N_SUBBUFS;
+
+ kmemtrace_chan = relay_open(NULL, NULL, KMEMTRACE_SUBBUF_SIZE,
+ kmemtrace_n_subbufs, &relay_callbacks,
+ NULL);
+ if (!kmemtrace_chan) {
+ printk(KERN_INFO "kmemtrace: could not open relay channel\n");
+ return;
+ }
+
+ 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);
+ if (err)
+ goto probe_alloc_fail;
+ err = marker_probe_register("kmemtrace_free", "type_id %d "
+ "call_site %lu ptr %lu",
+ kmemtrace_probe_free, NULL);
+ if (err)
+ goto probe_free_fail;
+
+ printk(KERN_INFO "kmemtrace: early init successful.\n");
+ return;
+
+probe_free_fail:
+ err = marker_probe_unregister("kmemtrace_alloc",
+ kmemtrace_probe_alloc, NULL);
+ printk(KERN_INFO "kmemtrace: could not register marker probes!\n");
+probe_alloc_fail:
+ relay_close(kmemtrace_chan);
+ kmemtrace_chan = NULL;
+}
+
--
1.5.6.1
This adds hooks for the SLAB allocator, to allow tracing with kmemtrace.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
include/linux/slab_def.h | 56 +++++++++++++++++++++++++++++++++++++-----
mm/slab.c | 61 +++++++++++++++++++++++++++++++++++++++++----
2 files changed, 104 insertions(+), 13 deletions(-)
diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index 39c3a5e..77b8045 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -14,6 +14,7 @@
#include <asm/page.h> /* kmalloc_sizes.h needs PAGE_SIZE */
#include <asm/cache.h> /* kmalloc_sizes.h needs L1_CACHE_BYTES */
#include <linux/compiler.h>
+#include <linux/kmemtrace.h>
/* Size description struct for general caches. */
struct cache_sizes {
@@ -28,8 +29,20 @@ extern struct cache_sizes malloc_sizes[];
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 *cachep, gfp_t flags);
+#else
+static inline void *kmem_cache_alloc_notrace(struct kmem_cache *cachep,
+ gfp_t flags)
+{
+ return kmem_cache_alloc(cachep, flags);
+}
+#endif
+
static inline void *kmalloc(size_t size, gfp_t flags)
{
+ void *ret;
+
if (__builtin_constant_p(size)) {
int i = 0;
@@ -50,10 +63,17 @@ static inline void *kmalloc(size_t size, gfp_t flags)
found:
#ifdef CONFIG_ZONE_DMA
if (flags & GFP_DMA)
- return kmem_cache_alloc(malloc_sizes[i].cs_dmacachep,
- flags);
+ ret = kmem_cache_alloc_notrace(
+ malloc_sizes[i].cs_dmacachep, flags);
+ else
#endif
- return kmem_cache_alloc(malloc_sizes[i].cs_cachep, flags);
+ ret = kmem_cache_alloc_notrace(
+ malloc_sizes[i].cs_cachep, flags);
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL, _THIS_IP_, ret,
+ size, malloc_sizes[i].cs_size, flags);
+
+ return ret;
}
return __kmalloc(size, flags);
}
@@ -62,8 +82,23 @@ found:
extern void *__kmalloc_node(size_t size, gfp_t flags, int node);
extern 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 *cachep,
+ gfp_t flags,
+ int nodeid);
+#else
+static inline void *kmem_cache_alloc_node_notrace(struct kmem_cache *cachep,
+ gfp_t flags,
+ int nodeid)
+{
+ return kmem_cache_alloc_node(cachep, flags, nodeid);
+}
+#endif
+
static inline void *kmalloc_node(size_t size, gfp_t flags, int node)
{
+ void *ret;
+
if (__builtin_constant_p(size)) {
int i = 0;
@@ -84,11 +119,18 @@ static inline void *kmalloc_node(size_t size, gfp_t flags, int node)
found:
#ifdef CONFIG_ZONE_DMA
if (flags & GFP_DMA)
- return kmem_cache_alloc_node(malloc_sizes[i].cs_dmacachep,
- flags, node);
+ ret = kmem_cache_alloc_node_notrace(
+ malloc_sizes[i].cs_dmacachep, flags, node);
+ else
#endif
- return kmem_cache_alloc_node(malloc_sizes[i].cs_cachep,
- flags, node);
+ ret = kmem_cache_alloc_node_notrace(
+ malloc_sizes[i].cs_cachep, flags, node);
+
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL, _THIS_IP_,
+ ret, size, malloc_sizes[i].cs_size,
+ flags, node);
+
+ return ret;
}
return __kmalloc_node(size, flags, node);
}
diff --git a/mm/slab.c b/mm/slab.c
index 046607f..e9a61ac 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -111,6 +111,7 @@
#include <linux/rtmutex.h>
#include <linux/reciprocal_div.h>
#include <linux/debugobjects.h>
+#include <linux/kmemtrace.h>
#include <asm/cacheflush.h>
#include <asm/tlbflush.h>
@@ -3621,10 +3622,23 @@ static inline void __cache_free(struct kmem_cache *cachep, void *objp)
*/
void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
{
- return __cache_alloc(cachep, flags, __builtin_return_address(0));
+ void *ret = __cache_alloc(cachep, flags, __builtin_return_address(0));
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
+ obj_size(cachep), obj_size(cachep), flags);
+
+ return ret;
}
EXPORT_SYMBOL(kmem_cache_alloc);
+#ifdef CONFIG_KMEMTRACE
+void *kmem_cache_alloc_notrace(struct kmem_cache *cachep, gfp_t flags)
+{
+ return __cache_alloc(cachep, flags, __builtin_return_address(0));
+}
+EXPORT_SYMBOL(kmem_cache_alloc_notrace);
+#endif
+
/**
* kmem_ptr_validate - check if an untrusted pointer might be a slab entry.
* @cachep: the cache we're checking against
@@ -3669,20 +3683,44 @@ out:
#ifdef CONFIG_NUMA
void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
{
- return __cache_alloc_node(cachep, flags, nodeid,
- __builtin_return_address(0));
+ 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), obj_size(cachep),
+ flags, nodeid);
+
+ return ret;
}
EXPORT_SYMBOL(kmem_cache_alloc_node);
+#ifdef CONFIG_KMEMTRACE
+void *kmem_cache_alloc_node_notrace(struct kmem_cache *cachep,
+ gfp_t flags,
+ int nodeid)
+{
+ return __cache_alloc_node(cachep, flags, nodeid,
+ __builtin_return_address(0));
+}
+EXPORT_SYMBOL(kmem_cache_alloc_node_notrace);
+#endif
+
static __always_inline void *
__do_kmalloc_node(size_t size, gfp_t flags, int node, void *caller)
{
struct kmem_cache *cachep;
+ void *ret;
cachep = kmem_find_general_cachep(size, flags);
if (unlikely(ZERO_OR_NULL_PTR(cachep)))
return cachep;
- return kmem_cache_alloc_node(cachep, flags, node);
+ ret = kmem_cache_alloc_node_notrace(cachep, flags, node);
+
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL,
+ (unsigned long) caller, ret,
+ size, cachep->buffer_size, flags, node);
+
+ return ret;
}
#ifdef CONFIG_DEBUG_SLAB
@@ -3718,6 +3756,7 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
void *caller)
{
struct kmem_cache *cachep;
+ void *ret;
/* If you want to save a few bytes .text space: replace
* __ with kmem_.
@@ -3727,11 +3766,17 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
cachep = __find_general_cachep(size, flags);
if (unlikely(ZERO_OR_NULL_PTR(cachep)))
return cachep;
- return __cache_alloc(cachep, flags, caller);
+ ret = __cache_alloc(cachep, flags, caller);
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL,
+ (unsigned long) caller, ret,
+ size, cachep->buffer_size, flags);
+
+ return ret;
}
-#ifdef CONFIG_DEBUG_SLAB
+#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_KMEMTRACE)
void *__kmalloc(size_t size, gfp_t flags)
{
return __do_kmalloc(size, flags, __builtin_return_address(0));
@@ -3770,6 +3815,8 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp)
debug_check_no_obj_freed(objp, obj_size(cachep));
__cache_free(cachep, objp);
local_irq_restore(flags);
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, objp);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -3796,6 +3843,8 @@ void kfree(const void *objp)
debug_check_no_obj_freed(objp, obj_size(c));
__cache_free(c, (void *)objp);
local_irq_restore(flags);
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_KERNEL, _RET_IP_, objp);
}
EXPORT_SYMBOL(kfree);
--
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 | 9 +++++++-
mm/slub.c | 47 ++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index d117ea2..0cef121 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 */
@@ -205,7 +206,13 @@ void *__kmalloc(size_t size, gfp_t flags);
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, order);
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL, _THIS_IP_, ret,
+ size, PAGE_SIZE << order, flags);
+
+ return ret;
}
static __always_inline void *kmalloc(size_t size, gfp_t flags)
diff --git a/mm/slub.c b/mm/slub.c
index 315c392..a6f930f 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:
@@ -1652,14 +1653,25 @@ 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));
+ void *ret = slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
+ s->objsize, s->size, gfpflags);
+
+ return ret;
}
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));
+ void *ret = slab_alloc(s, gfpflags, node,
+ __builtin_return_address(0));
+
+ 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
@@ -1771,6 +1783,8 @@ 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));
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, x);
}
EXPORT_SYMBOL(kmem_cache_free);
@@ -2676,6 +2690,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);
@@ -2685,7 +2700,12 @@ 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));
+ ret = slab_alloc(s, flags, -1, __builtin_return_address(0));
+
+ kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL, _RET_IP_, ret,
+ size, (size_t) s->size, (unsigned long) flags);
+
+ return ret;
}
EXPORT_SYMBOL(__kmalloc);
@@ -2704,16 +2724,29 @@ 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_KERNEL, _RET_IP_, ret,
+ size, PAGE_SIZE << get_order(size),
+ (unsigned long) flags, node);
+
+ return ret;
+ }
s = get_slab(size, flags);
if (unlikely(ZERO_OR_NULL_PTR(s)))
return s;
- return slab_alloc(s, flags, node, __builtin_return_address(0));
+ ret = slab_alloc(s, flags, node, __builtin_return_address(0));
+
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL, _RET_IP_, ret,
+ size, s->size, (unsigned long) flags, node);
+
+ return ret;
}
EXPORT_SYMBOL(__kmalloc_node);
#endif
@@ -2771,6 +2804,8 @@ void kfree(const void *x)
return;
}
slab_free(page->slab, page, object, __builtin_return_address(0));
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_KERNEL, _RET_IP_, x);
}
EXPORT_SYMBOL(kfree);
--
1.5.6.1
This adds hooks for the SLOB allocator, to allow tracing with kmemtrace.
Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
---
mm/slob.c | 37 +++++++++++++++++++++++++++++++------
1 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/mm/slob.c b/mm/slob.c
index a3ad667..0335c01 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -65,6 +65,7 @@
#include <linux/module.h>
#include <linux/rcupdate.h>
#include <linux/list.h>
+#include <linux/kmemtrace.h>
#include <asm/atomic.h>
/*
@@ -463,27 +464,38 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
{
unsigned int *m;
int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
+ void *ret;
if (size < PAGE_SIZE - align) {
if (!size)
return ZERO_SIZE_PTR;
m = slob_alloc(size + align, gfp, align, node);
+
if (!m)
return NULL;
*m = size;
- return (void *)m + align;
+ ret = (void *)m + align;
+
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL,
+ _RET_IP_, ret,
+ size, size + align, gfp, node);
} else {
- void *ret;
+ unsigned int order = get_order(size);
- ret = slob_new_page(gfp | __GFP_COMP, get_order(size), node);
+ ret = slob_new_page(gfp | __GFP_COMP, order, node);
if (ret) {
struct page *page;
page = virt_to_page(ret);
page->private = size;
}
- return ret;
+
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL,
+ _RET_IP_, ret,
+ size, PAGE_SIZE << order, gfp, node);
}
+
+ return ret;
}
EXPORT_SYMBOL(__kmalloc_node);
@@ -501,6 +513,8 @@ void kfree(const void *block)
slob_free(m, *m + align);
} else
put_page(&sp->page);
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_KERNEL, _RET_IP_, block);
}
EXPORT_SYMBOL(kfree);
@@ -569,10 +583,19 @@ void *kmem_cache_alloc_node(struct kmem_cache *c, gfp_t flags, int node)
{
void *b;
- if (c->size < PAGE_SIZE)
+ if (c->size < PAGE_SIZE) {
b = slob_alloc(c->size, flags, c->align, node);
- else
+ kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_CACHE,
+ _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);
+ }
if (c->ctor)
c->ctor(c, b);
@@ -608,6 +631,8 @@ void kmem_cache_free(struct kmem_cache *c, void *b)
} else {
__kmem_cache_free(b, c->size);
}
+
+ kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, b);
}
EXPORT_SYMBOL(kmem_cache_free);
--
1.5.6.1
Hi Eduard-Gabriel,
On Thu, Jul 17, 2008 at 3:46 AM, Eduard - Gabriel Munteanu
<[email protected]> wrote:
> This adds hooks for the SLAB allocator, to allow tracing with kmemtrace.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> ---
> include/linux/slab_def.h | 56 +++++++++++++++++++++++++++++++++++++-----
> mm/slab.c | 61 +++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 104 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
> index 39c3a5e..77b8045 100644
> --- a/include/linux/slab_def.h
> +++ b/include/linux/slab_def.h
> @@ -14,6 +14,7 @@
> #include <asm/page.h> /* kmalloc_sizes.h needs PAGE_SIZE */
> #include <asm/cache.h> /* kmalloc_sizes.h needs L1_CACHE_BYTES */
> #include <linux/compiler.h>
> +#include <linux/kmemtrace.h>
>
> /* Size description struct for general caches. */
> struct cache_sizes {
> @@ -28,8 +29,20 @@ extern struct cache_sizes malloc_sizes[];
> 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 *cachep, gfp_t flags);
> +#else
> +static inline void *kmem_cache_alloc_notrace(struct kmem_cache *cachep,
> + gfp_t flags)
> +{
> + return kmem_cache_alloc(cachep, flags);
> +}
> +#endif
> +
> static inline void *kmalloc(size_t size, gfp_t flags)
> {
> + void *ret;
> +
> if (__builtin_constant_p(size)) {
> int i = 0;
>
> @@ -50,10 +63,17 @@ static inline void *kmalloc(size_t size, gfp_t flags)
> found:
> #ifdef CONFIG_ZONE_DMA
> if (flags & GFP_DMA)
> - return kmem_cache_alloc(malloc_sizes[i].cs_dmacachep,
> - flags);
> + ret = kmem_cache_alloc_notrace(
> + malloc_sizes[i].cs_dmacachep, flags);
> + else
> #endif
> - return kmem_cache_alloc(malloc_sizes[i].cs_cachep, flags);
> + ret = kmem_cache_alloc_notrace(
> + malloc_sizes[i].cs_cachep, flags);
> +
> + kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL, _THIS_IP_, ret,
> + size, malloc_sizes[i].cs_size, flags);
We have malloc_sizes[i].cs_size here as the _allocated_ size (which
seems wrong to be btw).
> +
> + return ret;
> }
> return __kmalloc(size, flags);
> }
> @@ -62,8 +82,23 @@ found:
> extern void *__kmalloc_node(size_t size, gfp_t flags, int node);
> extern 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 *cachep,
> + gfp_t flags,
> + int nodeid);
> +#else
> +static inline void *kmem_cache_alloc_node_notrace(struct kmem_cache *cachep,
> + gfp_t flags,
> + int nodeid)
> +{
> + return kmem_cache_alloc_node(cachep, flags, nodeid);
> +}
> +#endif
> +
> static inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> {
> + void *ret;
> +
> if (__builtin_constant_p(size)) {
> int i = 0;
>
> @@ -84,11 +119,18 @@ static inline void *kmalloc_node(size_t size, gfp_t flags, int node)
> found:
> #ifdef CONFIG_ZONE_DMA
> if (flags & GFP_DMA)
> - return kmem_cache_alloc_node(malloc_sizes[i].cs_dmacachep,
> - flags, node);
> + ret = kmem_cache_alloc_node_notrace(
> + malloc_sizes[i].cs_dmacachep, flags, node);
> + else
> #endif
> - return kmem_cache_alloc_node(malloc_sizes[i].cs_cachep,
> - flags, node);
> + ret = kmem_cache_alloc_node_notrace(
> + malloc_sizes[i].cs_cachep, flags, node);
> +
> + kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL, _THIS_IP_,
> + ret, size, malloc_sizes[i].cs_size,
> + flags, node);
And here.
> +
> + return ret;
> }
> return __kmalloc_node(size, flags, node);
> }
>
> +#ifdef CONFIG_KMEMTRACE
> +void *kmem_cache_alloc_node_notrace(struct kmem_cache *cachep,
> + gfp_t flags,
> + int nodeid)
> +{
> + return __cache_alloc_node(cachep, flags, nodeid,
> + __builtin_return_address(0));
> +}
> +EXPORT_SYMBOL(kmem_cache_alloc_node_notrace);
> +#endif
> +
> static __always_inline void *
> __do_kmalloc_node(size_t size, gfp_t flags, int node, void *caller)
> {
> struct kmem_cache *cachep;
> + void *ret;
>
> cachep = kmem_find_general_cachep(size, flags);
> if (unlikely(ZERO_OR_NULL_PTR(cachep)))
> return cachep;
> - return kmem_cache_alloc_node(cachep, flags, node);
> + ret = kmem_cache_alloc_node_notrace(cachep, flags, node);
> +
> + kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL,
> + (unsigned long) caller, ret,
> + size, cachep->buffer_size, flags, node);
But here we use cachep->buffer_size and...
> +
> + return ret;
> }
>
> #ifdef CONFIG_DEBUG_SLAB
> @@ -3718,6 +3756,7 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
> void *caller)
> {
> struct kmem_cache *cachep;
> + void *ret;
>
> /* If you want to save a few bytes .text space: replace
> * __ with kmem_.
> @@ -3727,11 +3766,17 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
> cachep = __find_general_cachep(size, flags);
> if (unlikely(ZERO_OR_NULL_PTR(cachep)))
> return cachep;
> - return __cache_alloc(cachep, flags, caller);
> + ret = __cache_alloc(cachep, flags, caller);
> +
> + kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL,
> + (unsigned long) caller, ret,
> + size, cachep->buffer_size, flags);
...here as well. Why?
Also,
> diff --git a/mm/slab.c b/mm/slab.c
> index 046607f..e9a61ac 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -111,6 +111,7 @@
> #include <linux/rtmutex.h>
> #include <linux/reciprocal_div.h>
> #include <linux/debugobjects.h>
> +#include <linux/kmemtrace.h>
>
> #include <asm/cacheflush.h>
> #include <asm/tlbflush.h>
> @@ -3621,10 +3622,23 @@ static inline void __cache_free(struct kmem_cache *cachep, void *objp)
> */
> void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
> {
> - return __cache_alloc(cachep, flags, __builtin_return_address(0));
> + void *ret = __cache_alloc(cachep, flags, __builtin_return_address(0));
> +
> + kmemtrace_mark_alloc(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
> + obj_size(cachep), obj_size(cachep), flags);
Here....
> +
> + return ret;
> }
> EXPORT_SYMBOL(kmem_cache_alloc);
>
> +#ifdef CONFIG_KMEMTRACE
> +void *kmem_cache_alloc_notrace(struct kmem_cache *cachep, gfp_t flags)
> +{
> + return __cache_alloc(cachep, flags, __builtin_return_address(0));
> +}
> +EXPORT_SYMBOL(kmem_cache_alloc_notrace);
> +#endif
> +
> /**
> * kmem_ptr_validate - check if an untrusted pointer might be a slab entry.
> * @cachep: the cache we're checking against
> @@ -3669,20 +3683,44 @@ out:
> #ifdef CONFIG_NUMA
> void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
> {
> - return __cache_alloc_node(cachep, flags, nodeid,
> - __builtin_return_address(0));
> + 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), obj_size(cachep),
> + flags, nodeid);
...and here, we use obj_size().
> +
> + return ret;
> }
> EXPORT_SYMBOL(kmem_cache_alloc_node);
AFAICT, you should always use ->buffer_size as the_allocated_ size. Hmm?
Hi,
[Adding Matt as cc.]
On Thu, Jul 17, 2008 at 3:46 AM, Eduard - Gabriel Munteanu
<[email protected]> wrote:
> This adds hooks for the SLOB allocator, to allow tracing with kmemtrace.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
Reviewed-by: Pekka Enberg <[email protected]>
> ---
> mm/slob.c | 37 +++++++++++++++++++++++++++++++------
> 1 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/mm/slob.c b/mm/slob.c
> index a3ad667..0335c01 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -65,6 +65,7 @@
> #include <linux/module.h>
> #include <linux/rcupdate.h>
> #include <linux/list.h>
> +#include <linux/kmemtrace.h>
> #include <asm/atomic.h>
>
> /*
> @@ -463,27 +464,38 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
> {
> unsigned int *m;
> int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
> + void *ret;
>
> if (size < PAGE_SIZE - align) {
> if (!size)
> return ZERO_SIZE_PTR;
>
> m = slob_alloc(size + align, gfp, align, node);
> +
> if (!m)
> return NULL;
> *m = size;
> - return (void *)m + align;
> + ret = (void *)m + align;
> +
> + kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL,
> + _RET_IP_, ret,
> + size, size + align, gfp, node);
> } else {
> - void *ret;
> + unsigned int order = get_order(size);
>
> - ret = slob_new_page(gfp | __GFP_COMP, get_order(size), node);
> + ret = slob_new_page(gfp | __GFP_COMP, order, node);
> if (ret) {
> struct page *page;
> page = virt_to_page(ret);
> page->private = size;
> }
> - return ret;
> +
> + kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL,
> + _RET_IP_, ret,
> + size, PAGE_SIZE << order, gfp, node);
> }
> +
> + return ret;
> }
> EXPORT_SYMBOL(__kmalloc_node);
>
> @@ -501,6 +513,8 @@ void kfree(const void *block)
> slob_free(m, *m + align);
> } else
> put_page(&sp->page);
> +
> + kmemtrace_mark_free(KMEMTRACE_TYPE_KERNEL, _RET_IP_, block);
> }
> EXPORT_SYMBOL(kfree);
>
> @@ -569,10 +583,19 @@ void *kmem_cache_alloc_node(struct kmem_cache *c, gfp_t flags, int node)
> {
> void *b;
>
> - if (c->size < PAGE_SIZE)
> + if (c->size < PAGE_SIZE) {
> b = slob_alloc(c->size, flags, c->align, node);
> - else
> + kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_CACHE,
> + _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);
> + }
>
> if (c->ctor)
> c->ctor(c, b);
> @@ -608,6 +631,8 @@ void kmem_cache_free(struct kmem_cache *c, void *b)
> } else {
> __kmem_cache_free(b, c->size);
> }
> +
> + kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, b);
> }
> EXPORT_SYMBOL(kmem_cache_free);
>
> --
> 1.5.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Thu, Jul 17, 2008 at 3:46 AM, Eduard - Gabriel Munteanu
<[email protected]> wrote:
> 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 | 9 +++++++-
> mm/slub.c | 47 ++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
> index d117ea2..0cef121 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 */
> @@ -205,7 +206,13 @@ void *__kmalloc(size_t size, gfp_t flags);
>
> 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, order);
> +
> + kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL, _THIS_IP_, ret,
> + size, PAGE_SIZE << order, flags);
> +
> + return ret;
> }
>
> static __always_inline void *kmalloc(size_t size, gfp_t flags)
> diff --git a/mm/slub.c b/mm/slub.c
> index 315c392..a6f930f 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:
> @@ -1652,14 +1653,25 @@ 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));
> + void *ret = slab_alloc(s, gfpflags, -1, __builtin_return_address(0));
> +
> + kmemtrace_mark_alloc(KMEMTRACE_TYPE_CACHE, _RET_IP_, ret,
> + s->objsize, s->size, gfpflags);
> +
> + return ret;
> }
> 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));
> + void *ret = slab_alloc(s, gfpflags, node,
> + __builtin_return_address(0));
> +
> + 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
> @@ -1771,6 +1783,8 @@ 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));
> +
> + kmemtrace_mark_free(KMEMTRACE_TYPE_CACHE, _RET_IP_, x);
> }
> EXPORT_SYMBOL(kmem_cache_free);
>
> @@ -2676,6 +2690,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);
> @@ -2685,7 +2700,12 @@ 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));
> + ret = slab_alloc(s, flags, -1, __builtin_return_address(0));
> +
> + kmemtrace_mark_alloc(KMEMTRACE_TYPE_KERNEL, _RET_IP_, ret,
> + size, (size_t) s->size, (unsigned long) flags);
What are these casts doing here? I think you can just drop them, no?
> +
> + return ret;
> }
> EXPORT_SYMBOL(__kmalloc);
>
> @@ -2704,16 +2724,29 @@ 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_KERNEL, _RET_IP_, ret,
> + size, PAGE_SIZE << get_order(size),
> + (unsigned long) flags, node);
Don't cast flags to unsigned long. The kmemtrace core should use gfp_t
as the parameter type.
> +
> + return ret;
> + }
>
> s = get_slab(size, flags);
>
> if (unlikely(ZERO_OR_NULL_PTR(s)))
> return s;
>
> - return slab_alloc(s, flags, node, __builtin_return_address(0));
> + ret = slab_alloc(s, flags, node, __builtin_return_address(0));
> +
> + kmemtrace_mark_alloc_node(KMEMTRACE_TYPE_KERNEL, _RET_IP_, ret,
> + size, s->size, (unsigned long) flags, node);
Another cast here.
> +
> + return ret;
> }
> EXPORT_SYMBOL(__kmalloc_node);
> #endif
> @@ -2771,6 +2804,8 @@ void kfree(const void *x)
> return;
> }
> slab_free(page->slab, page, object, __builtin_return_address(0));
> +
> + kmemtrace_mark_free(KMEMTRACE_TYPE_KERNEL, _RET_IP_, x);
> }
> EXPORT_SYMBOL(kfree);
>
> --
> 1.5.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Hi,
[Adding Randy to cc for the Documentation/ parts and Matt for the core.]
On Thu, Jul 17, 2008 at 3:46 AM, Eduard - Gabriel Munteanu
<[email protected]> wrote:
> kmemtrace provides tracing for slab allocator functions, such as kmalloc,
> kfree, kmem_cache_alloc, kmem_cache_free etc.. Collected data is then fed
> to the userspace application in order to analyse allocation hotspots,
> internal fragmentation and so on, making it possible to see how well an
> allocator performs, as well as debug and profile kernel code.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> ---
> Documentation/kernel-parameters.txt | 6 +
> Documentation/vm/kmemtrace.txt | 96 ++++++++++++++++
> MAINTAINERS | 6 +
> include/linux/kmemtrace.h | 110 ++++++++++++++++++
> init/main.c | 2 +
> lib/Kconfig.debug | 4 +
> mm/Makefile | 2 +-
> mm/kmemtrace.c | 208 +++++++++++++++++++++++++++++++++++
> 8 files changed, 433 insertions(+), 1 deletions(-)
> create mode 100644 Documentation/vm/kmemtrace.txt
> create mode 100644 include/linux/kmemtrace.h
> create mode 100644 mm/kmemtrace.c
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index b52f47d..b230aff 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -49,6 +49,7 @@ parameter is applicable:
> ISAPNP ISA PnP code is enabled.
> ISDN Appropriate ISDN support is enabled.
> JOY Appropriate joystick support is enabled.
> + KMEMTRACE kmemtrace is enabled.
> LIBATA Libata driver is enabled
> LP Printer support is enabled.
> LOOP Loopback device support is enabled.
> @@ -941,6 +942,11 @@ and is between 256 and 4096 characters. It is defined in the file
> use the HighMem zone if it exists, and the Normal
> zone if it does not.
>
> + kmemtrace.subbufs=n [KNL,KMEMTRACE] Overrides the number of
> + subbufs kmemtrace's relay channel has. Set this
> + higher than default (KMEMTRACE_N_SUBBUFS in code) if
> + you experience buffer overruns.
> +
> movablecore=nn[KMG] [KNL,X86-32,IA-64,PPC,X86-64] This parameter
> is similar to kernelcore except it specifies the
> amount of memory used for migratable allocations.
> diff --git a/Documentation/vm/kmemtrace.txt b/Documentation/vm/kmemtrace.txt
> new file mode 100644
> index 0000000..1147ecb
> --- /dev/null
> +++ b/Documentation/vm/kmemtrace.txt
> @@ -0,0 +1,96 @@
> + kmemtrace - Kernel Memory Tracer
> +
> + by Eduard - Gabriel Munteanu
> + <[email protected]>
> +
A chapter on what kmemtrace is here would probably be helpful.
> +
> +I. Design and goals
> +===================
> +
> +kmemtrace was designed to handle rather large amounts of data. Thus, it uses
> +the relay interface to export whatever is logged to userspace, which then
> +stores it. Analysis and reporting is done asynchronously, that is, after the
> +data is collected and stored. By design, it allows one to log and analyse
> +on different machines and different arches.
> +
> +As this is a debugging feature, kmemtrace's ABI is not designed to be very
> +stable, although this may happen in the future if it's deemed mature and
> +sufficient. So the userspace tool does not contain a copy of the kernel
> +header. Instead, the ABI allows checking if the logged data matches the
> +userspace tool. Well, what I said about ABI stability isn't totally true:
> +while I've tried hard to cover all possible (and useful) use cases, I don't
> +want it frozen in the current state. I anticipate the ABI will be _quite_
> +stable, even across multiple stable kernel versions, but I don't make any
> +guarantees regarding this matter.
> +
> +Summary of design goals:
> + - allow logging and analysis to be done across different machines
> + - be fast and anticipate usage in high-load environments (*)
> + - be reasonably extensible
> + - have a _reasonably_ (not completely) stable ABI
> +
> +(*) - one of the reasons Pekka Enberg's original userspace data analysis
> + tool's code was rewritten from Perl to C (although this is more than a
> + simple conversion)
> +
> +
> +II. Quick usage guide
> +=====================
> +
> +1) Get a kernel that supports kmemtrace and build it accordingly (i.e. enable
> +CONFIG_KMEMTRACE).
> +
> +2) Get the userspace tool and build it:
> +$ git-clone git://repo.or.cz/kmemtrace-user.git # current repository
> +$ cd kmemtrace-user/
> +$ autoreconf
> +$ ./configure # Supply KERNEL_SOURCES=/path/to/sources/ if you're
> + # _not_ running this on a kmemtrace-enabled kernel.
> +$ make
As I mentioned in private, I would prefer we drop autoconf from the
userspace tool, but maybe that's just my personal preference.
> +
> +3) Boot the kmemtrace-enabled kernel if you haven't, preferably in the
> +'single' runlevel (so that relay buffers don't fill up easily), and run
> +kmemtrace:
> +# '$' does not mean user, but root here.
> +$ mount -t debugfs none /debug
> +$ mount -t proc none /proc
> +$ cd path/to/kmemtrace-user/
> +$ ./kmemtraced
> +Wait a bit, then stop it with CTRL+C.
> +$ cat /debug/kmemtrace/total_overruns # Check if we didn't overrun, should
> + # be zero.
> +$ (Optionally) [Run kmemtrace_check separately on each cpu[0-9]*.out file to
> + check its correctness]
> +$ ./kmemtrace-report
> +
> +Now you should have a nice and short summary of how the allocator performs.
> +
> +III. FAQ and known issues
> +=========================
> +Q: 'cat /debug/kmemtrace/total_overruns' is non-zero, how do I fix this?
> +Should I worry?
> +A: If it's non-zero, this affects kmemtrace's accuracy, depending on how
> +large the number is. You can fix it by supplying a higher
> +'kmemtrace.subbufs=N' kernel parameter.
> +---
> +
> +Q: kmemtrace_check reports errors, how do I fix this? Should I worry?
> +A: This is a bug and should be reported. It can occur for a variety of
> +reasons:
> + - possible bugs in relay code
> + - possible misuse of relay by kmemtrace
> + - timestamps being collected unorderly
> +Or you may fix it yourself and send us a patch.
> +---
> +
> +Q: kmemtrace_report shows many errors, how do I fix this? Should I worry?
> +A: This is a known issue and I'm working on it. These might be true errors
> +in kernel code, which may have inconsistent behavior (e.g. allocating memory
> +with kmem_cache_alloc() and freeing it with kfree()). Pekka Enberg pointed
> +out this behavior may work with SLAB, but may fail with other allocators.
> +
> +It may also be due to lack of tracing in some unusual allocator functions.
> +
> +We don't want bug reports regarding this issue yet.
> +---
I think you're supposed to document the actual filesystem in
Documentation/ABI as well.
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 56a2f67..e967bc2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2425,6 +2425,12 @@ M: [email protected]
> L: [email protected]
> S: Maintained
>
> +KMEMTRACE
> +P: Eduard - Gabriel Munteanu
> +M: [email protected]
> +L: [email protected]
> +S: Maintained
> +
> KPROBES
> P: Ananth N Mavinakayanahalli
> M: [email protected]
> diff --git a/include/linux/kmemtrace.h b/include/linux/kmemtrace.h
> new file mode 100644
> index 0000000..da69d22
> --- /dev/null
> +++ b/include/linux/kmemtrace.h
> @@ -0,0 +1,110 @@
> +/*
> + * Copyright (C) 2008 Eduard - Gabriel Munteanu
> + *
> + * This file is released under GPL version 2.
> + */
> +
> +#ifndef _LINUX_KMEMTRACE_H
> +#define _LINUX_KMEMTRACE_H
> +
> +#include <linux/types.h>
> +
> +/* ABI definition starts here. */
> +
> +#define KMEMTRACE_ABI_VERSION 1
> +
> +enum kmemtrace_event_id {
> + KMEMTRACE_EVENT_NULL = 0, /* Erroneous event. */
I don't think this is used anywhere so why not drop it?
> + KMEMTRACE_EVENT_ALLOC,
> + KMEMTRACE_EVENT_FREE,
> +};
> +
> +enum kmemtrace_type_id {
> + KMEMTRACE_TYPE_KERNEL = 0, /* kmalloc() / kfree(). */
> + KMEMTRACE_TYPE_CACHE, /* kmem_cache_*(). */
> + KMEMTRACE_TYPE_PAGES, /* __get_free_pages() and friends. */
I still think kernel vs. cache is confusing because both allocations
*are* for the kernel. So perhaps kmalloc vs. cache?
> +};
> +
> +struct kmemtrace_event {
So why don't we have the ABI version embedded here like blktrace has
so that user-space can check if the format matches its expectations?
That should be future-proof as well: as long as y ou keep the existing
fields where they're at now, you can always add new fields at the end
of the struct.
> + __u16 event_id; /* Allocate or free? */
> + __u16 type_id; /* Kind of allocation/free. */
> + __s32 node; /* Target CPU. */
> + __u64 call_site; /* Caller address. */
> + __u64 ptr; /* Pointer to allocation. */
> + __u64 bytes_req; /* Number of bytes requested. */
> + __u64 bytes_alloc; /* Number of bytes allocated. */
> + __u64 gfp_flags; /* Requested flags. */
> + __s64 timestamp; /* When the operation occured in ns. */
> +} __attribute__ ((__packed__));
> +
> +/* End of ABI definition. */
> +
> +#ifdef __KERNEL__
> +
> +#include <linux/marker.h>
> +
> +#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,
> + unsigned long 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,
> + bytes_req, bytes_alloc, 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,
> + unsigned long 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,
> + unsigned long gfp_flags)
> +{
> + kmemtrace_mark_alloc_node(type_id, call_site, ptr,
> + bytes_req, bytes_alloc, gfp_flags, -1);
> +}
> +
> +#endif /* __KERNEL__ */
> +
> +#endif /* _LINUX_KMEMTRACE_H */
> +
> diff --git a/init/main.c b/init/main.c
> index 057f364..c00659c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -66,6 +66,7 @@
> #include <asm/setup.h>
> #include <asm/sections.h>
> #include <asm/cacheflush.h>
> +#include <linux/kmemtrace.h>
>
> #ifdef CONFIG_X86_LOCAL_APIC
> #include <asm/smp.h>
> @@ -641,6 +642,7 @@ asmlinkage void __init start_kernel(void)
> enable_debug_pagealloc();
> cpu_hotplug_init();
> kmem_cache_init();
> + kmemtrace_init();
> debug_objects_mem_init();
> idr_init_cache();
> setup_per_cpu_pageset();
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d2099f4..6bacab5 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -674,6 +674,10 @@ config FIREWIRE_OHCI_REMOTE_DMA
>
> If unsure, say N.
>
> +config KMEMTRACE
> + bool "Kernel memory tracer"
> + depends on RELAY && DEBUG_FS && MARKERS
> +
> source "samples/Kconfig"
>
> source "lib/Kconfig.kgdb"
> diff --git a/mm/Makefile b/mm/Makefile
> index 18c143b..d88a3bc 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -33,4 +33,4 @@ obj-$(CONFIG_MIGRATION) += migrate.o
> obj-$(CONFIG_SMP) += allocpercpu.o
> obj-$(CONFIG_QUICKLIST) += quicklist.o
> obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o
> -
> +obj-$(CONFIG_KMEMTRACE) += kmemtrace.o
> diff --git a/mm/kmemtrace.c b/mm/kmemtrace.c
> new file mode 100644
> index 0000000..9258010
> --- /dev/null
> +++ b/mm/kmemtrace.c
> @@ -0,0 +1,208 @@
> +/*
> + * Copyright (C) 2008 Pekka Enberg, Eduard - Gabriel Munteanu
> + *
> + * This file is released under GPL version 2.
> + */
> +
> +#include <linux/string.h>
> +#include <linux/debugfs.h>
> +#include <linux/relay.h>
> +#include <linux/module.h>
> +#include <linux/marker.h>
> +#include <linux/gfp.h>
> +#include <linux/kmemtrace.h>
> +
> +#define KMEMTRACE_SUBBUF_SIZE (8192 * sizeof(struct kmemtrace_event))
> +#define KMEMTRACE_N_SUBBUFS 20
> +
> +static struct rchan *kmemtrace_chan;
> +static u32 kmemtrace_buf_overruns;
> +static unsigned int kmemtrace_n_subbufs;
> +
> +static inline void kmemtrace_log_event(struct kmemtrace_event *event)
> +{
> + relay_write(kmemtrace_chan, event, sizeof(struct kmemtrace_event));
> +}
> +
> +static void kmemtrace_probe_alloc(void *probe_data, void *call_data,
> + const char *format, va_list *args)
> +{
> + unsigned long flags;
> + struct kmemtrace_event ev;
> +
> + /*
> + * Don't convert this to use structure initializers,
> + * C99 does not guarantee the rvalues evaluation order.
> + */
> + ev.event_id = KMEMTRACE_EVENT_ALLOC;
> + ev.type_id = va_arg(*args, int);
> + ev.call_site = va_arg(*args, unsigned long);
> + ev.ptr = va_arg(*args, unsigned long);
> + /* Don't trace ignored allocations. */
> + if (!ev.ptr)
> + return;
> + ev.bytes_req = va_arg(*args, unsigned long);
> + ev.bytes_alloc = va_arg(*args, unsigned long);
> + /* ev.timestamp set below, to preserve event ordering. */
> + ev.gfp_flags = va_arg(*args, unsigned long);
> + ev.node = va_arg(*args, int);
> +
> + local_irq_save(flags);
Why do we disable local irqs here? (Perhaps a comment is in order.)
> + ev.timestamp = ktime_to_ns(ktime_get());
> + kmemtrace_log_event(&ev);
> + local_irq_restore(flags);
> +}
> +
> +static void kmemtrace_probe_free(void *probe_data, void *call_data,
> + const char *format, va_list *args)
> +{
> + unsigned long flags;
> + struct kmemtrace_event ev;
> +
> + /*
> + * Don't convert this to use structure initializers,
> + * C99 does not guarantee the rvalues evaluation order.
> + */
> + ev.event_id = KMEMTRACE_EVENT_FREE;
> + ev.type_id = va_arg(*args, int);
> + ev.call_site = va_arg(*args, unsigned long);
> + ev.ptr = va_arg(*args, unsigned long);
> + /* Don't trace ignored allocations. */
> + if (!ev.ptr)
> + return;
> + /* ev.timestamp set below, to preserve event ordering. */
> +
> + local_irq_save(flags);
(same here)
> + ev.timestamp = ktime_to_ns(ktime_get());
> + kmemtrace_log_event(&ev);
> + local_irq_restore(flags);
> +}
> +
> +static struct dentry *
> +kmemtrace_create_buf_file(const char *filename, struct dentry *parent,
> + int mode, struct rchan_buf *buf, int *is_global)
> +{
> + return debugfs_create_file(filename, mode, parent, buf,
> + &relay_file_operations);
> +}
> +
> +static int kmemtrace_remove_buf_file(struct dentry *dentry)
> +{
> + debugfs_remove(dentry);
> +
> + return 0;
> +}
> +
> +static int kmemtrace_count_overruns(struct rchan_buf *buf,
> + void *subbuf, void *prev_subbuf,
> + size_t prev_padding)
> +{
> + if (relay_buf_full(buf)) {
> + kmemtrace_buf_overruns++;
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static struct rchan_callbacks relay_callbacks = {
> + .create_buf_file = kmemtrace_create_buf_file,
> + .remove_buf_file = kmemtrace_remove_buf_file,
> + .subbuf_start = kmemtrace_count_overruns,
> +};
> +
> +static struct dentry *kmemtrace_dir;
> +static struct dentry *kmemtrace_overruns_dentry;
> +
> +static void kmemtrace_cleanup(void)
> +{
> + relay_close(kmemtrace_chan);
> + marker_probe_unregister("kmemtrace_alloc",
> + kmemtrace_probe_alloc, NULL);
> + marker_probe_unregister("kmemtrace_free",
> + kmemtrace_probe_free, NULL);
> + if (kmemtrace_overruns_dentry)
> + debugfs_remove(kmemtrace_overruns_dentry);
> +}
> +
> +static int __init kmemtrace_setup_late(void)
> +{
> + if (!kmemtrace_chan)
> + goto failed;
> +
> + kmemtrace_dir = debugfs_create_dir("kmemtrace", NULL);
> + if (!kmemtrace_dir)
> + goto cleanup;
> +
> + kmemtrace_overruns_dentry =
> + debugfs_create_u32("total_overruns", S_IRUSR,
> + kmemtrace_dir, &kmemtrace_buf_overruns);
> + if (!kmemtrace_overruns_dentry)
> + goto dir_cleanup;
> +
> + if (relay_late_setup_files(kmemtrace_chan, "cpu", kmemtrace_dir))
> + goto overrun_cleanup;
> +
> + printk(KERN_INFO "kmemtrace: fully up.\n");
> +
> + return 0;
> +
> +overrun_cleanup:
> + debugfs_remove(kmemtrace_overruns_dentry);
> + kmemtrace_overruns_dentry = NULL;
> +dir_cleanup:
> + debugfs_remove(kmemtrace_dir);
> +cleanup:
> + kmemtrace_cleanup();
> +failed:
> + return 1;
> +}
> +late_initcall(kmemtrace_setup_late);
> +
> +static int __init kmemtrace_set_subbuf_size(char *str)
> +{
> + get_option(&str, &kmemtrace_n_subbufs);
> + return 0;
> +}
> +early_param("kmemtrace.subbufs", kmemtrace_set_subbuf_size);
> +
> +void kmemtrace_init(void)
> +{
> + int err;
> +
> + if (!kmemtrace_n_subbufs)
> + kmemtrace_n_subbufs = KMEMTRACE_N_SUBBUFS;
> +
> + kmemtrace_chan = relay_open(NULL, NULL, KMEMTRACE_SUBBUF_SIZE,
> + kmemtrace_n_subbufs, &relay_callbacks,
> + NULL);
> + if (!kmemtrace_chan) {
> + printk(KERN_INFO "kmemtrace: could not open relay channel\n");
> + return;
> + }
> +
> + 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);
> + if (err)
> + goto probe_alloc_fail;
> + err = marker_probe_register("kmemtrace_free", "type_id %d "
> + "call_site %lu ptr %lu",
> + kmemtrace_probe_free, NULL);
> + if (err)
> + goto probe_free_fail;
> +
> + printk(KERN_INFO "kmemtrace: early init successful.\n");
> + return;
> +
> +probe_free_fail:
> + err = marker_probe_unregister("kmemtrace_alloc",
> + kmemtrace_probe_alloc, NULL);
> + printk(KERN_INFO "kmemtrace: could not register marker probes!\n");
> +probe_alloc_fail:
> + relay_close(kmemtrace_chan);
> + kmemtrace_chan = NULL;
> +}
> +
> --
> 1.5.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Thu, 2008-07-17 at 10:43 +0300, Pekka Enberg wrote:
> Hi,
>
> [Adding Matt as cc.]
>
> On Thu, Jul 17, 2008 at 3:46 AM, Eduard - Gabriel Munteanu
> <[email protected]> wrote:
> > This adds hooks for the SLOB allocator, to allow tracing with kmemtrace.
> >
> > Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
>
> Reviewed-by: Pekka Enberg <[email protected]>
Acked-by: Matt Mackall <[email protected]>
--
Mathematics is the supreme nostalgia of our time.
On Thu, Jul 17, 2008 at 10:46:51AM +0300, Pekka Enberg wrote:
> On Thu, Jul 17, 2008 at 3:46 AM, Eduard - Gabriel Munteanu
> <[email protected]> wrote:
> > 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 | 9 +++++++-
> > mm/slub.c | 47 ++++++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 49 insertions(+), 7 deletions(-)
>
> What are these casts doing here? I think you can just drop them, no?
Yes, I should cut all these casts off. Will resubmit soon.
Eduard
On Thu, Jul 17, 2008 at 11:01:21AM +0300, Pekka Enberg wrote:
> Hi,
>
> [Adding Randy to cc for the Documentation/ parts and Matt for the core.]
>
> On Thu, Jul 17, 2008 at 3:46 AM, Eduard - Gabriel Munteanu
> <[email protected]> wrote:
> > diff --git a/Documentation/vm/kmemtrace.txt b/Documentation/vm/kmemtrace.txt
> > new file mode 100644
> > index 0000000..1147ecb
> > --- /dev/null
> > +++ b/Documentation/vm/kmemtrace.txt
> > @@ -0,0 +1,96 @@
> > + kmemtrace - Kernel Memory Tracer
> > +
> > + by Eduard - Gabriel Munteanu
> > + <[email protected]>
> > +
>
> A chapter on what kmemtrace is here would probably be helpful.
>
Will do.
> > +2) Get the userspace tool and build it:
> > +$ git-clone git://repo.or.cz/kmemtrace-user.git # current repository
> > +$ cd kmemtrace-user/
> > +$ autoreconf
> > +$ ./configure # Supply KERNEL_SOURCES=/path/to/sources/ if you're
> > + # _not_ running this on a kmemtrace-enabled kernel.
> > +$ make
>
> As I mentioned in private, I would prefer we drop autoconf from the
> userspace tool, but maybe that's just my personal preference.
>
Yes, I'm working on a legible plain Makefile. However, I'd leave both
the autoconf variant and the plain Makefile in the package for now. Most
developers can use autoconf since it's part of the standard toolset for
regular userspace.
> > +Q: kmemtrace_report shows many errors, how do I fix this? Should I worry?
> > +A: This is a known issue and I'm working on it. These might be true errors
> > +in kernel code, which may have inconsistent behavior (e.g. allocating memory
> > +with kmem_cache_alloc() and freeing it with kfree()). Pekka Enberg pointed
> > +out this behavior may work with SLAB, but may fail with other allocators.
> > +
> > +It may also be due to lack of tracing in some unusual allocator functions.
> > +
> > +We don't want bug reports regarding this issue yet.
> > +---
>
> I think you're supposed to document the actual filesystem in
> Documentation/ABI as well.
Sounds like a good idea, I'll get on it.
> > +enum kmemtrace_event_id {
> > + KMEMTRACE_EVENT_NULL = 0, /* Erroneous event. */
>
> I don't think this is used anywhere so why not drop it?
>
We keep this here because we see all-zeros events when relay errors
occur. I'd like to keep it until I'm sure the relay problem was solved
(although I've not seen these errors in a while since I patched
kmemtraced).
> > + KMEMTRACE_EVENT_ALLOC,
> > + KMEMTRACE_EVENT_FREE,
> > +};
> > +
> > +enum kmemtrace_type_id {
> > + KMEMTRACE_TYPE_KERNEL = 0, /* kmalloc() / kfree(). */
> > + KMEMTRACE_TYPE_CACHE, /* kmem_cache_*(). */
> > + KMEMTRACE_TYPE_PAGES, /* __get_free_pages() and friends. */
>
> I still think kernel vs. cache is confusing because both allocations
> *are* for the kernel. So perhaps kmalloc vs. cache?
>
Okay, will s/TYPE_KERNEL/TYPE_KMALLOC/.
> > +};
> > +
> > +struct kmemtrace_event {
>
> So why don't we have the ABI version embedded here like blktrace has
> so that user-space can check if the format matches its expectations?
> That should be future-proof as well: as long as y ou keep the existing
> fields where they're at now, you can always add new fields at the end
> of the struct.
>
You can't add fields at the end, because the struct size will change and
reads will be erroneous. Also, stamping every 'packet' with ABI version
looks like a huge waste of space.
> > + __u16 event_id; /* Allocate or free? */
> > + __u16 type_id; /* Kind of allocation/free. */
> > + __s32 node; /* Target CPU. */
> > + __u64 call_site; /* Caller address. */
> > + __u64 ptr; /* Pointer to allocation. */
> > + __u64 bytes_req; /* Number of bytes requested. */
> > + __u64 bytes_alloc; /* Number of bytes allocated. */
> > + __u64 gfp_flags; /* Requested flags. */
> > + __s64 timestamp; /* When the operation occured in ns. */
> > +} __attribute__ ((__packed__));
> > +
> > + ev.bytes_req = va_arg(*args, unsigned long);
> > + ev.bytes_alloc = va_arg(*args, unsigned long);
> > + /* ev.timestamp set below, to preserve event ordering. */
> > + ev.gfp_flags = va_arg(*args, unsigned long);
> > + ev.node = va_arg(*args, int);
> > +
> > + local_irq_save(flags);
>
> Why do we disable local irqs here? (Perhaps a comment is in order.)
>
We do it to preserve ordering of timestamps. Otherwise, the CPU might
get preempted (by IRQs or otherwise) and the event might not be logged
in the order timestamps were taken.
I thought the previous comment about 'ev.timestamp' was enough. I'll
make things more explicit.
> > + ev.timestamp = ktime_to_ns(ktime_get());
> > + kmemtrace_log_event(&ev);
> > + local_irq_restore(flags);
> > +}
> > +
> > +static void kmemtrace_probe_free(void *probe_data, void *call_data,
> > + const char *format, va_list *args)
> > +{
> > + unsigned long flags;
> > + struct kmemtrace_event ev;
> > +
> > + /*
> > + * Don't convert this to use structure initializers,
> > + * C99 does not guarantee the rvalues evaluation order.
> > + */
> > + ev.event_id = KMEMTRACE_EVENT_FREE;
> > + ev.type_id = va_arg(*args, int);
> > + ev.call_site = va_arg(*args, unsigned long);
> > + ev.ptr = va_arg(*args, unsigned long);
> > + /* Don't trace ignored allocations. */
> > + if (!ev.ptr)
> > + return;
> > + /* ev.timestamp set below, to preserve event ordering. */
> > +
> > + local_irq_save(flags);
>
> (same here)
>
> > + ev.timestamp = ktime_to_ns(ktime_get());
> > + kmemtrace_log_event(&ev);
> > + local_irq_restore(flags);
> > +}
> > +
> > +static struct dentry *
> > +kmemtrace_create_buf_file(const char *filename, struct dentry *parent,
> > + int mode, struct rchan_buf *buf, int *is_global)
> > +{
> > + return debugfs_create_file(filename, mode, parent, buf,
> > + &relay_file_operations);
> > +}
> > +
> > +static int kmemtrace_remove_buf_file(struct dentry *dentry)
> > +{
> > + debugfs_remove(dentry);
> > +
> > + return 0;
> > +}
> > +
> > +static int kmemtrace_count_overruns(struct rchan_buf *buf,
> > + void *subbuf, void *prev_subbuf,
> > + size_t prev_padding)
> > +{
> > + if (relay_buf_full(buf)) {
> > + kmemtrace_buf_overruns++;
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
> > +
> > +static struct rchan_callbacks relay_callbacks = {
> > + .create_buf_file = kmemtrace_create_buf_file,
> > + .remove_buf_file = kmemtrace_remove_buf_file,
> > + .subbuf_start = kmemtrace_count_overruns,
> > +};
> > +
> > +static struct dentry *kmemtrace_dir;
> > +static struct dentry *kmemtrace_overruns_dentry;
> > +
> > +static void kmemtrace_cleanup(void)
> > +{
> > + relay_close(kmemtrace_chan);
> > + marker_probe_unregister("kmemtrace_alloc",
> > + kmemtrace_probe_alloc, NULL);
> > + marker_probe_unregister("kmemtrace_free",
> > + kmemtrace_probe_free, NULL);
> > + if (kmemtrace_overruns_dentry)
> > + debugfs_remove(kmemtrace_overruns_dentry);
> > +}
> > +
> > +static int __init kmemtrace_setup_late(void)
> > +{
> > + if (!kmemtrace_chan)
> > + goto failed;
> > +
> > + kmemtrace_dir = debugfs_create_dir("kmemtrace", NULL);
> > + if (!kmemtrace_dir)
> > + goto cleanup;
> > +
> > + kmemtrace_overruns_dentry =
> > + debugfs_create_u32("total_overruns", S_IRUSR,
> > + kmemtrace_dir, &kmemtrace_buf_overruns);
> > + if (!kmemtrace_overruns_dentry)
> > + goto dir_cleanup;
> > +
> > + if (relay_late_setup_files(kmemtrace_chan, "cpu", kmemtrace_dir))
> > + goto overrun_cleanup;
> > +
> > + printk(KERN_INFO "kmemtrace: fully up.\n");
> > +
> > + return 0;
> > +
> > +overrun_cleanup:
> > + debugfs_remove(kmemtrace_overruns_dentry);
> > + kmemtrace_overruns_dentry = NULL;
> > +dir_cleanup:
> > + debugfs_remove(kmemtrace_dir);
> > +cleanup:
> > + kmemtrace_cleanup();
> > +failed:
> > + return 1;
> > +}
> > +late_initcall(kmemtrace_setup_late);
> > +
> > +static int __init kmemtrace_set_subbuf_size(char *str)
> > +{
> > + get_option(&str, &kmemtrace_n_subbufs);
> > + return 0;
> > +}
> > +early_param("kmemtrace.subbufs", kmemtrace_set_subbuf_size);
> > +
> > +void kmemtrace_init(void)
> > +{
> > + int err;
> > +
> > + if (!kmemtrace_n_subbufs)
> > + kmemtrace_n_subbufs = KMEMTRACE_N_SUBBUFS;
> > +
> > + kmemtrace_chan = relay_open(NULL, NULL, KMEMTRACE_SUBBUF_SIZE,
> > + kmemtrace_n_subbufs, &relay_callbacks,
> > + NULL);
> > + if (!kmemtrace_chan) {
> > + printk(KERN_INFO "kmemtrace: could not open relay channel\n");
> > + return;
> > + }
> > +
> > + 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);
> > + if (err)
> > + goto probe_alloc_fail;
> > + err = marker_probe_register("kmemtrace_free", "type_id %d "
> > + "call_site %lu ptr %lu",
> > + kmemtrace_probe_free, NULL);
> > + if (err)
> > + goto probe_free_fail;
> > +
> > + printk(KERN_INFO "kmemtrace: early init successful.\n");
> > + return;
> > +
> > +probe_free_fail:
> > + err = marker_probe_unregister("kmemtrace_alloc",
> > + kmemtrace_probe_alloc, NULL);
> > + printk(KERN_INFO "kmemtrace: could not register marker probes!\n");
> > +probe_alloc_fail:
> > + relay_close(kmemtrace_chan);
> > + kmemtrace_chan = NULL;
> > +}
> > +
> > --
> > 1.5.6.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
On Thu, 17 Jul 2008 03:46:45 +0300 Eduard - Gabriel Munteanu wrote:
> kmemtrace provides tracing for slab allocator functions, such as kmalloc,
> kfree, kmem_cache_alloc, kmem_cache_free etc.. Collected data is then fed
> to the userspace application in order to analyse allocation hotspots,
> internal fragmentation and so on, making it possible to see how well an
> allocator performs, as well as debug and profile kernel code.
>
> Signed-off-by: Eduard - Gabriel Munteanu <[email protected]>
> ---
> Documentation/kernel-parameters.txt | 6 +
> Documentation/vm/kmemtrace.txt | 96 ++++++++++++++++
> MAINTAINERS | 6 +
> include/linux/kmemtrace.h | 110 ++++++++++++++++++
> init/main.c | 2 +
> lib/Kconfig.debug | 4 +
> mm/Makefile | 2 +-
> mm/kmemtrace.c | 208 +++++++++++++++++++++++++++++++++++
> 8 files changed, 433 insertions(+), 1 deletions(-)
> create mode 100644 Documentation/vm/kmemtrace.txt
> create mode 100644 include/linux/kmemtrace.h
> create mode 100644 mm/kmemtrace.c
>
> diff --git a/Documentation/vm/kmemtrace.txt b/Documentation/vm/kmemtrace.txt
> new file mode 100644
> index 0000000..1147ecb
> --- /dev/null
> +++ b/Documentation/vm/kmemtrace.txt
> +II. Quick usage guide
> +=====================
> +
> +1) Get a kernel that supports kmemtrace and build it accordingly (i.e. enable
> +CONFIG_KMEMTRACE).
> +
> +2) Get the userspace tool and build it:
> +$ git-clone git://repo.or.cz/kmemtrace-user.git # current repository
> +$ cd kmemtrace-user/
> +$ autoreconf
> +$ ./configure # Supply KERNEL_SOURCES=/path/to/sources/ if you're
> + # _not_ running this on a kmemtrace-enabled kernel.
> +$ make
> +
> +3) Boot the kmemtrace-enabled kernel if you haven't, preferably in the
> +'single' runlevel (so that relay buffers don't fill up easily), and run
> +kmemtrace:
> +# '$' does not mean user, but root here.
> +$ mount -t debugfs none /debug
Please mount at /sys/kernel/debug, i.e., the expected debugfs mount point.
> +$ mount -t proc none /proc
> +$ cd path/to/kmemtrace-user/
> +$ ./kmemtraced
> +Wait a bit, then stop it with CTRL+C.
> +$ cat /debug/kmemtrace/total_overruns # Check if we didn't overrun, should
> + # be zero.
> +$ (Optionally) [Run kmemtrace_check separately on each cpu[0-9]*.out file to
> + check its correctness]
> +$ ./kmemtrace-report
> +
> +Now you should have a nice and short summary of how the allocator performs.
Otherwise looks nice. Thanks.
---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/
On Thu, Jul 17, 2008 at 02:34:34PM -0700, Randy Dunlap wrote:
> On Thu, 17 Jul 2008 03:46:45 +0300 Eduard - Gabriel Munteanu wrote:
>
> > +3) Boot the kmemtrace-enabled kernel if you haven't, preferably in the
> > +'single' runlevel (so that relay buffers don't fill up easily), and run
> > +kmemtrace:
> > +# '$' does not mean user, but root here.
> > +$ mount -t debugfs none /debug
>
> Please mount at /sys/kernel/debug, i.e., the expected debugfs mount point.
>
Oh, I did not know that. Thanks, will change accordingly.
> > +$ mount -t proc none /proc
> > +$ cd path/to/kmemtrace-user/
> > +$ ./kmemtraced
> > +Wait a bit, then stop it with CTRL+C.
> > +$ cat /debug/kmemtrace/total_overruns # Check if we didn't overrun, should
> > + # be zero.
> > +$ (Optionally) [Run kmemtrace_check separately on each cpu[0-9]*.out file to
> > + check its correctness]
> > +$ ./kmemtrace-report
> > +
> > +Now you should have a nice and short summary of how the allocator performs.
>
>
> Otherwise looks nice. Thanks.
>
> ---
> ~Randy
> Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
> http://linuxplumbersconf.org/
Hi Eduard-Gabriel,
On Thu, 17 Jul 2008, Eduard - Gabriel Munteanu wrote:
> > > +struct kmemtrace_event {
> >
> > So why don't we have the ABI version embedded here like blktrace has
> > so that user-space can check if the format matches its expectations?
> > That should be future-proof as well: as long as y ou keep the existing
> > fields where they're at now, you can always add new fields at the end
> > of the struct.
>
> You can't add fields at the end, because the struct size will change and
> reads will be erroneous. Also, stamping every 'packet' with ABI version
> looks like a huge waste of space.
It's an ABI so you want to make it backwards compatible and extensible.
Yes, it's just for debugging, so the rules are bit more relaxed here but
that's not an excuse for not designing the ABI properly.
I really wish we would follow the example set by blktrace here. It uses a
fixed-length header that knows the length of the rest of the packet.
Pekka
On Fri, Jul 18, 2008 at 11:48:03AM +0300, Pekka J Enberg wrote:
> Hi Eduard-Gabriel,
>
> On Thu, 17 Jul 2008, Eduard - Gabriel Munteanu wrote:
> > > > +struct kmemtrace_event {
> > >
> > > So why don't we have the ABI version embedded here like blktrace has
> > > so that user-space can check if the format matches its expectations?
> > > That should be future-proof as well: as long as y ou keep the existing
> > > fields where they're at now, you can always add new fields at the end
> > > of the struct.
> >
> > You can't add fields at the end, because the struct size will change and
> > reads will be erroneous. Also, stamping every 'packet' with ABI version
> > looks like a huge waste of space.
>
> It's an ABI so you want to make it backwards compatible and extensible.
> Yes, it's just for debugging, so the rules are bit more relaxed here but
> that's not an excuse for not designing the ABI properly.
I do expect to keep things source-compatible, but even
binary-compatible? Developers debug and write patches on the latest kernel,
not on a 6-month-old kernel. Isn't it reasonable that they would
recompile kmemtrace along with the kernel?
I would deem one ABI or another stable, but then we have to worry about
not breaking it, which leads to either bloating the kernel, or keeping
improvements away from kmemtrace. Should we do it just because this is an ABI?
> I really wish we would follow the example set by blktrace here. It uses a
> fixed-length header that knows the length of the rest of the packet.
I'd rather export the header length through a separate debugfs entry,
rather than add this to every packet. I don't think we need variable
length packets, unless we intend to export the whole stack trace, for
example.
By the way, do you anticipate the need for such a stack trace? It would seem
nice, but is it worth the trouble? (/me writes this down as a possible
future improvement)
> Pekka
Hi Eduard-Gabriel,
On Fri, Jul 18, 2008 at 11:48:03AM +0300, Pekka J Enberg wrote:
>> It's an ABI so you want to make it backwards compatible and extensible.
>> Yes, it's just for debugging, so the rules are bit more relaxed here but
>> that's not an excuse for not designing the ABI properly.
On Fri, Jul 18, 2008 at 1:13 PM, Eduard - Gabriel Munteanu
<[email protected]> wrote:
> I do expect to keep things source-compatible, but even
> binary-compatible? Developers debug and write patches on the latest kernel,
> not on a 6-month-old kernel. Isn't it reasonable that they would
> recompile kmemtrace along with the kernel?
Yes, I do think it's unreasonable. I, for one, am hoping distributions
will pick up the kmemtrace userspace at some point after which I don't
need to ever compile it myself.
On Fri, Jul 18, 2008 at 1:13 PM, Eduard - Gabriel Munteanu
<[email protected]> wrote:
> I would deem one ABI or another stable, but then we have to worry about
> not breaking it, which leads to either bloating the kernel, or keeping
> improvements away from kmemtrace. Should we do it just because this is an ABI?
Like I've said before, it's debugging/tracing infrastructure so the
rules are bit more relaxed. That said, what we should do is (1) make
the ABI as future-proof as we can, (2) explicitly mark it as unstable
by documenting it in Documentation/ABI/testing and (3) at some point
in time move it in Documentation/ABI/stable and hopefully never break
it again. But sure, we probably don't need to keep any "bloat" around
like we do with the syscall interface, for example.
And hopefully, the ABI is good enough to allow adding *new* tracing
events while retaining the old ones nicely in a backwards compatible
way.
On Fri, Jul 18, 2008 at 11:48:03AM +0300, Pekka J Enberg wrote:
>> I really wish we would follow the example set by blktrace here. It uses a
>> fixed-length header that knows the length of the rest of the packet.
On Fri, Jul 18, 2008 at 1:13 PM, Eduard - Gabriel Munteanu
<[email protected]> wrote:
> I'd rather export the header length through a separate debugfs entry,
> rather than add this to every packet. I don't think we need variable
> length packets, unless we intend to export the whole stack trace, for
> example.
Sure, makes sense.
Pekka
On Fri, Jul 18, 2008 at 05:38:04PM +0300, Pekka Enberg wrote:
> Hi Eduard-Gabriel,
> > I do expect to keep things source-compatible, but even
> > binary-compatible? Developers debug and write patches on the latest kernel,
> > not on a 6-month-old kernel. Isn't it reasonable that they would
> > recompile kmemtrace along with the kernel?
>
> Yes, I do think it's unreasonable. I, for one, am hoping distributions
> will pick up the kmemtrace userspace at some point after which I don't
> need to ever compile it myself.
Ok, I agree it's nice to have it in distros. I wasn't planning for this,
but it's good to know others' expectations.
Then I'll also add a turn-off mechanism, so maybe it makes it into distro
kernels too (either debug or not). And we don't need to include kernel
headers from userspace anymore and I'll just provide a copy.
BTW, I also expect the kmemtrace-user git repo to become stable soon
(i.e. no more revision history rewrites).
> On Fri, Jul 18, 2008 at 1:13 PM, Eduard - Gabriel Munteanu
> <[email protected]> wrote:
> > I would deem one ABI or another stable, but then we have to worry about
> > not breaking it, which leads to either bloating the kernel, or keeping
> > improvements away from kmemtrace. Should we do it just because this is an ABI?
>
> Like I've said before, it's debugging/tracing infrastructure so the
> rules are bit more relaxed. That said, what we should do is (1) make
> the ABI as future-proof as we can, (2) explicitly mark it as unstable
> by documenting it in Documentation/ABI/testing and (3) at some point
> in time move it in Documentation/ABI/stable and hopefully never break
> it again. But sure, we probably don't need to keep any "bloat" around
> like we do with the syscall interface, for example.
>
> And hopefully, the ABI is good enough to allow adding *new* tracing
> events while retaining the old ones nicely in a backwards compatible
> way.
Sounds like a good plan. I'll also update the docs (Documentation/ABI/ and
Documentation/vm/kmemtrace.txt) to reflect this.
> On Fri, Jul 18, 2008 at 11:48:03AM +0300, Pekka J Enberg wrote:
> >> I really wish we would follow the example set by blktrace here. It uses a
> >> fixed-length header that knows the length of the rest of the packet.
>
> On Fri, Jul 18, 2008 at 1:13 PM, Eduard - Gabriel Munteanu
> <[email protected]> wrote:
> > I'd rather export the header length through a separate debugfs entry,
> > rather than add this to every packet. I don't think we need variable
> > length packets, unless we intend to export the whole stack trace, for
> > example.
>
> Sure, makes sense.
> Pekka
On Fri, 2008-07-18 at 22:40 +0300, Eduard - Gabriel Munteanu wrote:
> On Fri, Jul 18, 2008 at 05:38:04PM +0300, Pekka Enberg wrote:
> > Hi Eduard-Gabriel,
> > > I do expect to keep things source-compatible, but even
> > > binary-compatible? Developers debug and write patches on the latest kernel,
> > > not on a 6-month-old kernel. Isn't it reasonable that they would
> > > recompile kmemtrace along with the kernel?
> >
> > Yes, I do think it's unreasonable. I, for one, am hoping distributions
> > will pick up the kmemtrace userspace at some point after which I don't
> > need to ever compile it myself.
>
> Ok, I agree it's nice to have it in distros. I wasn't planning for this,
> but it's good to know others' expectations.
It's worth pointing out that this is one of the big downfalls of things
like systemtap. If a tool can't just work out of the box for a distro,
it's basically a non-starter for most users.
--
Mathematics is the supreme nostalgia of our time.