Hello.
What do you think about ability to 'watch' leaked region? (hex + ascii).
(done via lib/hexdump.c)
To turn on hex dump:
echo "hexdump=on" > /sys/kernel/debug/kmemleak
/**
Or (as alternative):
echo "hexdump=f6aac7f8" > /sys/kernel/debug/kmemleak
where f6aac7f8 - object's pointer.
**/
cat /sys/kernel/debug/kmemleak
unreferenced object 0xf6aac7f8 (size 32):
comm "swapper", pid 1, jiffies 4294877610
HEX dump:
70 6e 70 20 30 30 3a 30 61 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 pnp 00:0a.ZZZZZZZZZZZZZZZZZZZZZ.
backtrace:
[<c10e92eb>] kmemleak_alloc+0x11b/0x2b0
[<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
[<c12c424e>] reserve_range+0x3e/0x1b0
[<c12c4454>] system_pnp_probe+0x94/0x140
[<c12baf84>] pnp_device_probe+0x84/0x100
[<c12f1919>] driver_probe_device+0x89/0x170
[<c12f1a99>] __driver_attach+0x99/0xa0
[<c12f1028>] bus_for_each_dev+0x58/0x90
[<c12f1764>] driver_attach+0x24/0x40
[<c12f0804>] bus_add_driver+0xc4/0x290
[<c12f1e10>] driver_register+0x70/0x130
[<c12bacd6>] pnp_register_driver+0x26/0x40
[<c15d4620>] pnp_system_init+0x1b/0x2e
[<c100115f>] do_one_initcall+0x3f/0x1a0
[<c15aa4af>] kernel_init+0x13e/0x1a6
[<c1003e07>] kernel_thread_helper+0x7/0x10
unreferenced object 0xf63f4d18 (size 192):
comm "swapper", pid 1, jiffies 4294878292
HEX dump:
3c 06 00 00 00 00 00 00 78 69 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 <.......xi......................
25 0c 00 00 00 00 00 00 25 0c 00 00 00 00 00 00 32 05 00 00 00 00 00 00 60 54 00 00 00 00 00 00 %.......%.......2.......`T......
0a 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 1f 0a 00 00 00 00 00 00 1f 0a 00 00 00 00 00 00 ................................
28 04 00 00 00 00 00 00 48 3f 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 (.......H?......................
19 08 00 00 00 00 00 00 19 08 00 00 00 00 00 00 1e 03 00 00 00 00 00 00 30 2a 00 00 00 00 00 00 ........................0*......
0a 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 13 06 00 00 00 00 00 00 13 06 00 00 00 00 00 00 ................................
backtrace:
[<c10e92eb>] kmemleak_alloc+0x11b/0x2b0
[<c10e584d>] __kmalloc+0x16d/0x210
[<c12b5757>] acpi_processor_register_performance+0x28e/0x468
[<c1016797>] acpi_cpufreq_cpu_init+0x97/0x560
[<c134d802>] cpufreq_add_dev+0x122/0x580
[<c12efd47>] sysdev_driver_register+0xa7/0x140
[<c134ca4e>] cpufreq_register_driver+0x9e/0x170
[<c15b4e0f>] acpi_cpufreq_init+0x8b/0xcd
[<c100115f>] do_one_initcall+0x3f/0x1a0
[<c15aa4af>] kernel_init+0x13e/0x1a6
[<c1003e07>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
To disable hex dump:
echo "hexdump=off" > /sys/kernel/debug/kmemleak
I guess it could safe someone's time.
(May be, showed examples aren't so good. Just to demonstrate the idea.)
(concept. feel free to ask for comments.)
diff -u -p
--- kmemleak.c 2009-06-28 20:18:59.000000000 +0300
+++ linux-2.6-sergey/mm/kmemleak.c 2009-06-28 20:21:29.000000000 +0300
@@ -160,6 +160,13 @@ struct kmemleak_object {
/* flag set to not scan the object */
#define OBJECT_NO_SCAN (1 << 2)
+/* number of bytes to print per line; must be 16 or 32 */
+#define HEX_ROW_SIZE 32
+/* number of bytes to print at a time (1, 2, 4, 8) */
+#define HEX_GROUP_SIZE 1
+/* include ASCII after the hex output */
+#define HEX_ASCII 1
+
/* the list of all allocated objects */
static LIST_HEAD(object_list);
/* the list of gray-colored objects (see color_gray comment below) */
@@ -182,6 +189,9 @@ static atomic_t kmemleak_early_log = ATO
/* set if a fata kmemleak error has occurred */
static atomic_t kmemleak_error = ATOMIC_INIT(0);
+/* set if HEX dump should be printed */
+static atomic_t kmemleak_hex_dump = ATOMIC_INIT(0);
+
/* minimum and maximum address that may be valid pointers */
static unsigned long min_addr = ULONG_MAX;
static unsigned long max_addr;
@@ -290,6 +300,29 @@ static int unreferenced_object(struct km
jiffies_last_scan);
}
+
+static void object_hex_dump(struct seq_file *seq, struct kmemleak_object *object)
+{
+ const u8 *ptr = (const u8*)object->pointer;
+ int len = object->size;
+ int i, linelen, remaining = object->size;
+ unsigned char linebuf[200];
+
+ seq_printf(seq, "HEX dump:\n");
+
+ for (i = 0; i < len; i += HEX_ROW_SIZE) {
+ linelen = min(remaining, HEX_ROW_SIZE);
+ remaining -= HEX_ROW_SIZE;
+ hex_dump_to_buffer(ptr + i, linelen, HEX_ROW_SIZE, HEX_GROUP_SIZE,
+ linebuf, sizeof(linebuf), HEX_ASCII);
+
+ seq_printf(seq, "%s\n", linebuf);
+ }
+
+ seq_printf(seq, "\n");
+}
+
+
/*
* Printing of the unreferenced objects information to the seq file. The
* print_unreferenced function must be called with the object->lock held.
@@ -301,10 +334,17 @@ static void print_unreferenced(struct se
seq_printf(seq, "unreferenced object 0x%08lx (size %zu):\n",
object->pointer, object->size);
+
seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
object->comm, object->pid, object->jiffies);
- seq_printf(seq, " backtrace:\n");
+ /* check whether hex dump should be printed*/
+ if (atomic_read(&kmemleak_hex_dump))
+ object_hex_dump(seq, object);
+
+
+ seq_printf(seq, " backtrace:\n");
+
for (i = 0; i < object->trace_len; i++) {
void *ptr = (void *)object->trace[i];
seq_printf(seq, " [<%p>] %pS\n", ptr, ptr);
@@ -1269,6 +1309,12 @@ static ssize_t kmemleak_write(struct fil
start_scan_thread();
else if (strncmp(buf, "scan=off", 8) == 0)
stop_scan_thread();
+ else if (strncmp(buf, "hexdump=on", 10) == 0) {
+ atomic_set(&kmemleak_hex_dump, 1);
+ }
+ else if (strncmp(buf, "hexdump=off", 11) == 0) {
+ atomic_set(&kmemleak_hex_dump, 0);
+ }
else if (strncmp(buf, "scan=", 5) == 0) {
unsigned long secs;
int err;
Sergey
Hi Sergey,
On Sun, Jun 28, 2009 at 8:36 PM, Sergey
Senozhatsky<[email protected]> wrote:
> What do you think about ability to 'watch' leaked region? (hex + ascii).
> (done via lib/hexdump.c)
What's your use case for this? I'm usually more interested in the
stack trace when there's a memory leak.
Pekka
On Mon, 2009-06-29 at 12:43 +0300, Pekka Enberg wrote:
> On Sun, Jun 28, 2009 at 8:36 PM, Sergey
> Senozhatsky<[email protected]> wrote:
> > What do you think about ability to 'watch' leaked region? (hex + ascii).
> > (done via lib/hexdump.c)
>
> What's your use case for this? I'm usually more interested in the
> stack trace when there's a memory leak.
I once had a need for such feature when investigating a memory leak (it
was more like debugging kmemleak) but a script combining dd, od
and /dev/kmem did the trick (I also work in an embedded world where I
have a halting debugger connected most of the times).
--
Catalin
On (06/29/09 12:43), Pekka Enberg wrote:
> Hi Sergey,
>
> On Sun, Jun 28, 2009 at 8:36 PM, Sergey
> Senozhatsky<[email protected]> wrote:
> > What do you think about ability to 'watch' leaked region? (hex + ascii).
> > (done via lib/hexdump.c)
>
> What's your use case for this? I'm usually more interested in the
> stack trace when there's a memory leak.
>
> Pekka
>
Hello Pekka,
Well, it's not easy to come up with something strong.
I agree, that stack gives you almost all you need.
HEX dump can give you a _tip_ in case you're not sure.
for example:
unreferenced object 0xf6aac7f8 (size 32):
comm "swapper", pid 1, jiffies 4294877610
HEX dump:
70 6e 70 20 30 30 3a 30 61 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 pnp 00:0a.ZZZZZZZZZZZZZZZZZZZZZ.
backtrace:
[<c10e92eb>] kmemleak_alloc+0x11b/0x2b0
[<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
[<c12c424e>] reserve_range+0x3e/0x1b0
[<c12c4454>] system_pnp_probe+0x94/0x140
[<c12baf84>] pnp_device_probe+0x84/0x100
[<c12f1919>] driver_probe_device+0x89/0x170
[<c12f1a99>] __driver_attach+0x99/0xa0
[<c12f1028>] bus_for_each_dev+0x58/0x90
[<c12f1764>] driver_attach+0x24/0x40
[<c12f0804>] bus_add_driver+0xc4/0x290
[<c12f1e10>] driver_register+0x70/0x130
[<c12bacd6>] pnp_register_driver+0x26/0x40
[<c15d4620>] pnp_system_init+0x1b/0x2e
[<c100115f>] do_one_initcall+0x3f/0x1a0
[<c15aa4af>] kernel_init+0x13e/0x1a6
[<c1003e07>] kernel_thread_helper+0x7/0x10
- Ah, pnp 00:0a. Got it.
or
- Ah, pnp 00:0a. No.. It's false. (EXAMPLE)
Or something like that :-)
Sergey
Hi Sergey,
On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> Well, it's not easy to come up with something strong.
> I agree, that stack gives you almost all you need.
>
> HEX dump can give you a _tip_ in case you're not sure.
Don't get me wrong, I'm not against it in any way. If Catalin is
interested in merging this kind of functionality, go for it! You might
want to consider unconditionally enabling the hexdump. If the
information is valuable, we should print it all the time.
Pekka
On Mon, 2009-06-29 at 13:19 +0300, Pekka Enberg wrote:
> On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> > Well, it's not easy to come up with something strong.
> > I agree, that stack gives you almost all you need.
> >
> > HEX dump can give you a _tip_ in case you're not sure.
>
> Don't get me wrong, I'm not against it in any way. If Catalin is
> interested in merging this kind of functionality, go for it! You might
> want to consider unconditionally enabling the hexdump. If the
> information is valuable, we should print it all the time.
Though I prefer to do as much as possible in user space, I think this
feature would be useful.
Anyway, I may not include it before the next merging window (when is
actually the best time for new features). Currently, my main focus is on
reducing the false positives.
--
Catalin
Hi Catalin,
On Mon, 2009-06-29 at 11:38 +0100, Catalin Marinas wrote:
> Anyway, I may not include it before the next merging window (when is
> actually the best time for new features). Currently, my main focus is on
> reducing the false positives.
Yes, this new feature shouldn't go into 2.6.31. That said, you _do_ want
to include it in linux-next now if you're interested in pushing it to
2.6.32.
Pekka
On (06/29/09 13:19), Pekka Enberg wrote:
> Hi Sergey,
>
> On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> > Well, it's not easy to come up with something strong.
> > I agree, that stack gives you almost all you need.
> >
> > HEX dump can give you a _tip_ in case you're not sure.
>
> Don't get me wrong, I'm not against it in any way.
I don't think so ;) _IF_ even so - it's absolutely normal, to my mind.
> If Catalin is interested in merging this kind of
> functionality, go for it! You might want to consider unconditionally
> enabling the hexdump. If the information is valuable, we should print
> it all the time.
I guess it's valuable enougth to print it, but not valuable enougth to print it all the time.
So, let me say - it's valuable enougth to print 'on demand', I guess. (I may be wrong).
BTW, printing it all the time we can spam kmemleak (in case there are objects sized 2K, 4K and so on).
That's why I wrote about hexdump=OBJECT_POINTER.
>
> Pekka
>
Sergey
On (06/29/09 11:38), Catalin Marinas wrote:
> On Mon, 2009-06-29 at 13:19 +0300, Pekka Enberg wrote:
> > On Mon, 2009-06-29 at 13:19 +0300, Sergey Senozhatsky wrote:
> > > Well, it's not easy to come up with something strong.
> > > I agree, that stack gives you almost all you need.
> > >
> > > HEX dump can give you a _tip_ in case you're not sure.
> >
> > Don't get me wrong, I'm not against it in any way. If Catalin is
> > interested in merging this kind of functionality, go for it! You might
> > want to consider unconditionally enabling the hexdump. If the
> > information is valuable, we should print it all the time.
>
> Though I prefer to do as much as possible in user space,
Agreed. Good example is 'function filtering' ;)
> I think this feature would be useful.
>
So, I'll continue my work. (given patch didn't even passed ./checkpatch.pl).
Ok?
> Anyway, I may not include it before the next merging window (when is
> actually the best time for new features). Currently, my main focus is on
> reducing the false positives.
Sure. No problems.
>
> --
> Catalin
>
Sergey
On Mon, 2009-06-29 at 13:45 +0300, Sergey Senozhatsky wrote:
> BTW, printing it all the time we can spam kmemleak (in case there are objects sized 2K, 4K and so on).
> That's why I wrote about hexdump=OBJECT_POINTER.
I'm more in favour of an on/off hexdump feature (maybe even permanently
on) and with a limit to the number of bytes it displays. For larger
blocks, the hexdump=OBJECT_POINTER is easily achievable in user space
via /dev/kmem.
My proposal is for an always on hexdump but with no more than 2-3 lines
of hex values. As Pekka said, I should get it into linux-next before the
next merging window.
--
Catalin
On (06/29/09 11:58), Catalin Marinas wrote:
> On Mon, 2009-06-29 at 13:45 +0300, Sergey Senozhatsky wrote:
> > BTW, printing it all the time we can spam kmemleak (in case there are objects sized 2K, 4K and so on).
> > That's why I wrote about hexdump=OBJECT_POINTER.
>
> I'm more in favour of an on/off hexdump feature (maybe even permanently
> on) and with a limit to the number of bytes it displays. For larger
> blocks, the hexdump=OBJECT_POINTER is easily achievable in user space
> via /dev/kmem.
>
Yeah. Good point.
> My proposal is for an always on hexdump but with no more than 2-3 lines
> of hex values.
I like it.
> As Pekka said, I should get it into linux-next before the
> next merging window.
I'll send new patch to you (today evening)/(tomorrow).
Ok?
>
> --
> Catalin
>
Sergey
Hello.
This is actually draft. We'll discuss details during next merge window (or earlier).
Thanks.
---
hex dump prints not more than HEX_MAX_LINES lines by HEX_ROW_SIZE (16 or 32) bytes.
( min(object->size, HEX_MAX_LINES * HEX_ROW_SIZE) ).
Example (HEX_ROW_SIZE 16):
unreferenced object 0xf68b59b8 (size 32):
comm "swapper", pid 1, jiffies 4294877610
hex dump (first 32 bytes):
70 6e 70 20 30 30 3a 30 31 00 5a 5a 5a 5a 5a 5a pnp 00:01.ZZZZZZ
5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZZZZZZZZZZZZZ.
backtrace:
[<c10e931b>] kmemleak_alloc+0x11b/0x2b0
[<c10e4bc1>] kmem_cache_alloc+0x111/0x1c0
[<c12c426e>] reserve_range+0x3e/0x1b0
[<c12c4474>] system_pnp_probe+0x94/0x140
[<c12bafa4>] pnp_device_probe+0x84/0x100
[<c12f1939>] driver_probe_device+0x89/0x170
[<c12f1ab9>] __driver_attach+0x99/0xa0
[<c12f1048>] bus_for_each_dev+0x58/0x90
[<c12f1784>] driver_attach+0x24/0x40
[<c12f0824>] bus_add_driver+0xc4/0x290
[<c12f1e30>] driver_register+0x70/0x130
[<c12bacf6>] pnp_register_driver+0x26/0x40
[<c15d671c>] pnp_system_init+0x1b/0x2e
[<c100115f>] do_one_initcall+0x3f/0x1a0
[<c15ac4af>] kernel_init+0x13e/0x1a6
[<c1003e07>] kernel_thread_helper+0x7/0x10
Example (HEX_ROW_SIZE 32):
unreferenced object 0xf5e2e130 (size 2048):
comm "modprobe", pid 2084, jiffies 4294880769
hex dump (first 64 bytes):
24 97 ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff $...............................
fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff fc ff ff ff ................................
backtrace:
[<c10e931b>] kmemleak_alloc+0x11b/0x2b0
[<c10e587d>] __kmalloc+0x16d/0x210
[<c10e73cd>] pcpu_mem_alloc+0x2d/0x80
[<c10e7482>] pcpu_extend_area_map+0x62/0x100
[<c10e7837>] pcpu_alloc+0x317/0x480
[<c10e79f7>] __alloc_percpu+0x17/0x30
[<c122fd54>] alloc_disk_node+0x54/0x150
[<c122fe6c>] alloc_disk+0x1c/0x40
[<fd61a25b>] loop_alloc+0x6b/0x170 [loop]
[<fd61e092>] 0xfd61e092
[<c100115f>] do_one_initcall+0x3f/0x1a0
[<c10785ad>] sys_init_module+0xdd/0x220
[<c100324b>] sysenter_do_call+0x12/0x22
[<ffffffff>] 0xffffffff
unreferenced object 0xf5d3cd00 (size 32):
comm "consolechars", pid 2433, jiffies 4294894598
hex dump (first 32 bytes):
60 15 58 c1 00 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 `.X.....ZZZZZZZZZZZZZZZZZZZZZZZ.
backtrace:
[<c10e931b>] kmemleak_alloc+0x11b/0x2b0
[<c10e4bc1>] kmem_cache_alloc+0x111/0x1c0
[<c12cf49b>] tty_ldisc_try_get+0x2b/0x130
[<c12cf7d7>] tty_ldisc_get+0x37/0x70
[<c12cfa54>] tty_ldisc_reinit+0x34/0x70
[<c12cfac5>] tty_ldisc_release+0x35/0x60
[<c12ca1fe>] tty_release_dev+0x33e/0x500
[<c12ca3e0>] tty_release+0x20/0x40
[<c10ed61d>] __fput+0xed/0x200
[<c10ed752>] fput+0x22/0x40
[<c10e96c9>] filp_close+0x49/0x90
[<c1045445>] put_files_struct+0xb5/0xe0
[<c10454b5>] exit_files+0x45/0x60
[<c1046fd3>] do_exit+0x133/0x6c0
[<c10475a5>] do_group_exit+0x45/0xa0
[<c1047622>] sys_exit_group+0x22/0x40
hexdump.c can print prefix:
case DUMP_PREFIX_ADDRESS:
f6998df0: 3c 06 00 00 00 00 00 00 78 69 00 00 .... <.......xi......................
case DUMP_PREFIX_OFFSET:
00000000: 3c 06 00 00 00 00 00 00 78 69 00 00 .... <.......xi......................
default:
3c 06 00 00 00 00 00 00 78 69 00 00 00 00 00 .... <.......xi......................
hex_dump_object(struct seq_file *seq, struct kmemleak_object *object)
can be extended to accept prefix flag either. Though I don't think it's so important. So, it prints without any prefix.
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 5063873..65c5d74 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -160,6 +160,15 @@ struct kmemleak_object {
/* flag set to not scan the object */
#define OBJECT_NO_SCAN (1 << 2)
+/* number of bytes to print per line; must be 16 or 32 */
+#define HEX_ROW_SIZE 32
+/* number of bytes to print at a time (1, 2, 4, 8) */
+#define HEX_GROUP_SIZE 1
+/* include ASCII after the hex output */
+#define HEX_ASCII 1
+/* max number of lines to be printed */
+#define HEX_MAX_LINES 2
+
/* the list of all allocated objects */
static LIST_HEAD(object_list);
/* the list of gray-colored objects (see color_gray comment below) */
@@ -181,6 +190,8 @@ static atomic_t kmemleak_initialized = ATOMIC_INIT(0);
static atomic_t kmemleak_early_log = ATOMIC_INIT(1);
/* set if a fata kmemleak error has occurred */
static atomic_t kmemleak_error = ATOMIC_INIT(0);
+/* set if hex dump should be printed */
+static atomic_t kmemleak_hex_dump = ATOMIC_INIT(1);
/* minimum and maximum address that may be valid pointers */
static unsigned long min_addr = ULONG_MAX;
@@ -258,6 +269,35 @@ static void kmemleak_disable(void);
kmemleak_disable(); \
} while (0)
+
+/*
+ * Printing of the objects hex dump to the seq file. The number on lines
+ * to be printed is limited to HEX_MAX_LINES to prevent seq file spamming.
+ * The actual number of printed bytes depends on HEX_ROW_SIZE.
+ * It must be called with the object->lock held.
+ */
+static void hex_dump_object(struct seq_file *seq,
+ struct kmemleak_object *object)
+{
+ const u8 *ptr = (const u8 *)object->pointer;
+ /* Limit the number of lines to HEX_MAX_LINES. */
+ int len = min(object->size, (size_t)(HEX_MAX_LINES * HEX_ROW_SIZE));
+ int i, remaining = len;
+ unsigned char linebuf[200];
+
+ seq_printf(seq, " hex dump (first %d bytes):\n", len);
+
+ for (i = 0; i < len; i += HEX_ROW_SIZE) {
+ int linelen = min(remaining, HEX_ROW_SIZE);
+ remaining -= HEX_ROW_SIZE;
+ hex_dump_to_buffer(ptr + i, linelen, HEX_ROW_SIZE,
+ HEX_GROUP_SIZE, linebuf,
+ sizeof(linebuf), HEX_ASCII);
+
+ seq_printf(seq, " %s\n", linebuf);
+ }
+}
+
/*
* Object colors, encoded with count and min_count:
* - white - orphan object, not enough references to it (count < min_count)
@@ -303,6 +343,11 @@ static void print_unreferenced(struct seq_file *seq,
object->pointer, object->size);
seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
object->comm, object->pid, object->jiffies);
+
+ /* check whether hex dump should be printed */
+ if (atomic_read(&kmemleak_hex_dump))
+ hex_dump_object(seq, object);
+
seq_printf(seq, " backtrace:\n");
for (i = 0; i < object->trace_len; i++) {
@@ -1269,6 +1314,10 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
start_scan_thread();
else if (strncmp(buf, "scan=off", 8) == 0)
stop_scan_thread();
+ else if (strncmp(buf, "hexdump=on", 10) == 0)
+ atomic_set(&kmemleak_hex_dump, 1);
+ else if (strncmp(buf, "hexdump=off", 11) == 0)
+ atomic_set(&kmemleak_hex_dump, 0);
else if (strncmp(buf, "scan=", 5) == 0) {
unsigned long secs;
int err;
Sergey
Hi,
On Mon, 2009-06-29 at 21:10 +0100, Sergey Senozhatsky wrote:
> This is actually draft. We'll discuss details during next merge window (or earlier).
Better earlier (I plan to get some more kmemleak patches into
linux-next).
> hex dump prints not more than HEX_MAX_LINES lines by HEX_ROW_SIZE (16 or 32) bytes.
> ( min(object->size, HEX_MAX_LINES * HEX_ROW_SIZE) ).
>
> Example (HEX_ROW_SIZE 16):
>
> unreferenced object 0xf68b59b8 (size 32):
> comm "swapper", pid 1, jiffies 4294877610
> hex dump (first 32 bytes):
> 70 6e 70 20 30 30 3a 30 31 00 5a 5a 5a 5a 5a 5a pnp 00:01.ZZZZZZ
> 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZZZZZZZZZZZZZ.
That's my preferred as I do not want to go beyond column 80.
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 5063873..65c5d74 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -160,6 +160,15 @@ struct kmemleak_object {
> /* flag set to not scan the object */
> #define OBJECT_NO_SCAN (1 << 2)
>
> +/* number of bytes to print per line; must be 16 or 32 */
> +#define HEX_ROW_SIZE 32
16 here.
> +/* number of bytes to print at a time (1, 2, 4, 8) */
> +#define HEX_GROUP_SIZE 1
> +/* include ASCII after the hex output */
> +#define HEX_ASCII 1
> +/* max number of lines to be printed */
> +#define HEX_MAX_LINES 2
> +
> /* the list of all allocated objects */
> static LIST_HEAD(object_list);
> /* the list of gray-colored objects (see color_gray comment below) */
> @@ -181,6 +190,8 @@ static atomic_t kmemleak_initialized = ATOMIC_INIT(0);
> static atomic_t kmemleak_early_log = ATOMIC_INIT(1);
> /* set if a fata kmemleak error has occurred */
> static atomic_t kmemleak_error = ATOMIC_INIT(0);
> +/* set if hex dump should be printed */
> +static atomic_t kmemleak_hex_dump = ATOMIC_INIT(1);
[...]
> @@ -303,6 +343,11 @@ static void print_unreferenced(struct seq_file *seq,
> object->pointer, object->size);
> seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
> object->comm, object->pid, object->jiffies);
> +
> + /* check whether hex dump should be printed */
> + if (atomic_read(&kmemleak_hex_dump))
> + hex_dump_object(seq, object);
No need for this check, just leave it in all cases (as we now only read
the reports via the debug/kmemleak file.
> @@ -1269,6 +1314,10 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
> start_scan_thread();
> else if (strncmp(buf, "scan=off", 8) == 0)
> stop_scan_thread();
> + else if (strncmp(buf, "hexdump=on", 10) == 0)
> + atomic_set(&kmemleak_hex_dump, 1);
> + else if (strncmp(buf, "hexdump=off", 11) == 0)
> + atomic_set(&kmemleak_hex_dump, 0);
Same here.
Thanks.
--
Catalin
On (07/14/09 11:07), Catalin Marinas wrote:
> Hi,
>
Hello Catalin,
> On Mon, 2009-06-29 at 21:10 +0100, Sergey Senozhatsky wrote:
> > This is actually draft. We'll discuss details during next merge window (or earlier).
>
> Better earlier (I plan to get some more kmemleak patches into
> linux-next).
>
> > hex dump prints not more than HEX_MAX_LINES lines by HEX_ROW_SIZE (16 or 32) bytes.
> > ( min(object->size, HEX_MAX_LINES * HEX_ROW_SIZE) ).
> >
> > Example (HEX_ROW_SIZE 16):
> >
> > unreferenced object 0xf68b59b8 (size 32):
> > comm "swapper", pid 1, jiffies 4294877610
> > hex dump (first 32 bytes):
> > 70 6e 70 20 30 30 3a 30 31 00 5a 5a 5a 5a 5a 5a pnp 00:01.ZZZZZZ
> > 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZZZZZZZZZZZZZ.
>
> That's my preferred as I do not want to go beyond column 80.
>
Same with me.
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index 5063873..65c5d74 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -160,6 +160,15 @@ struct kmemleak_object {
> > /* flag set to not scan the object */
> > #define OBJECT_NO_SCAN (1 << 2)
> >
> > +/* number of bytes to print per line; must be 16 or 32 */
> > +#define HEX_ROW_SIZE 32
>
> 16 here.
>
OK.
[...]
> > @@ -303,6 +343,11 @@ static void print_unreferenced(struct seq_file *seq,
> > object->pointer, object->size);
> > seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
> > object->comm, object->pid, object->jiffies);
> > +
> > + /* check whether hex dump should be printed */
> > + if (atomic_read(&kmemleak_hex_dump))
> > + hex_dump_object(seq, object);
>
> No need for this check, just leave it in all cases (as we now only read
> the reports via the debug/kmemleak file.
>
> > @@ -1269,6 +1314,10 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
> > start_scan_thread();
> > else if (strncmp(buf, "scan=off", 8) == 0)
> > stop_scan_thread();
> > + else if (strncmp(buf, "hexdump=on", 10) == 0)
> > + atomic_set(&kmemleak_hex_dump, 1);
> > + else if (strncmp(buf, "hexdump=off", 11) == 0)
> > + atomic_set(&kmemleak_hex_dump, 0);
>
> Same here.
>
Am I understand correct that no way for user to on/off hexdump?
/* no need for atomic_t kmemleak_hex_dump */
> Thanks.
>
> --
> Catalin
>
Sergey
On Tue, 2009-07-14 at 13:33 +0300, Sergey Senozhatsky wrote:
> On (07/14/09 11:07), Catalin Marinas wrote:
> Am I understand correct that no way for user to on/off hexdump?
> /* no need for atomic_t kmemleak_hex_dump */
Yes. Two lines aren't really too much so we can always have them
displayed.
--
Catalin
On (07/14/09 11:34), Catalin Marinas wrote:
> On Tue, 2009-07-14 at 13:33 +0300, Sergey Senozhatsky wrote:
> > On (07/14/09 11:07), Catalin Marinas wrote:
> > Am I understand correct that no way for user to on/off hexdump?
> > /* no need for atomic_t kmemleak_hex_dump */
>
> Yes. Two lines aren't really too much so we can always have them
> displayed.
>
Agree.
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 5aabd41..f7b74ac 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -161,6 +161,15 @@ struct kmemleak_object {
/* flag set on newly allocated objects */
#define OBJECT_NEW (1 << 3)
+/* number of bytes to print per line; must be 16 or 32 */
+#define HEX_ROW_SIZE 16
+/* number of bytes to print at a time (1, 2, 4, 8) */
+#define HEX_GROUP_SIZE 1
+/* include ASCII after the hex output */
+#define HEX_ASCII 1
+/* max number of lines to be printed */
+#define HEX_MAX_LINES 2
+
/* the list of all allocated objects */
static LIST_HEAD(object_list);
/* the list of gray-colored objects (see color_gray comment below) */
@@ -254,6 +263,35 @@ static void kmemleak_disable(void);
kmemleak_disable(); \
} while (0)
+
+/*
+ * Printing of the objects hex dump to the seq file. The number on lines
+ * to be printed is limited to HEX_MAX_LINES to prevent seq file spamming.
+ * The actual number of printed bytes depends on HEX_ROW_SIZE.
+ * It must be called with the object->lock held.
+ */
+static void hex_dump_object(struct seq_file *seq,
+ struct kmemleak_object *object)
+{
+ const u8 *ptr = (const u8 *)object->pointer;
+ /* Limit the number of lines to HEX_MAX_LINES. */
+ int len = min(object->size, (size_t)(HEX_MAX_LINES * HEX_ROW_SIZE));
+ int i, remaining = len;
+ unsigned char linebuf[200];
+
+ seq_printf(seq, " hex dump (first %d bytes):\n", len);
+
+ for (i = 0; i < len; i += HEX_ROW_SIZE) {
+ int linelen = min(remaining, HEX_ROW_SIZE);
+ remaining -= HEX_ROW_SIZE;
+ hex_dump_to_buffer(ptr + i, linelen, HEX_ROW_SIZE,
+ HEX_GROUP_SIZE, linebuf,
+ sizeof(linebuf), HEX_ASCII);
+
+ seq_printf(seq, " %s\n", linebuf);
+ }
+}
+
/*
* Object colors, encoded with count and min_count:
* - white - orphan object, not enough references to it (count < min_count)
@@ -304,6 +342,9 @@ static void print_unreferenced(struct seq_file *seq,
object->pointer, object->size);
seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
object->comm, object->pid, object->jiffies);
+
+ hex_dump_object(seq, object);
+
seq_printf(seq, " backtrace:\n");
for (i = 0; i < object->trace_len; i++) {
On Tue, 2009-07-14 at 13:57 +0300, Sergey Senozhatsky wrote:
[...]
> +/*
> + * Printing of the objects hex dump to the seq file. The number on lines
> + * to be printed is limited to HEX_MAX_LINES to prevent seq file spamming.
> + * The actual number of printed bytes depends on HEX_ROW_SIZE.
> + * It must be called with the object->lock held.
> + */
[...]
The patch looks fine. Could you please add a description and
Signed-off-by line?
Thanks.
--
Catalin
On (07/14/09 14:39), Catalin Marinas wrote:
> On Tue, 2009-07-14 at 13:57 +0300, Sergey Senozhatsky wrote:
> [...]
> > +/*
> > + * Printing of the objects hex dump to the seq file. The number on lines
> > + * to be printed is limited to HEX_MAX_LINES to prevent seq file spamming.
> > + * The actual number of printed bytes depends on HEX_ROW_SIZE.
> > + * It must be called with the object->lock held.
> > + */
> [...]
>
> The patch looks fine. Could you please add a description and
> Signed-off-by line?
>
Sure. During 30-40 minutes (sorry, I'm a bit busy now). OK?
Should I update Documentation/kmemeleak.txt either?
> Thanks.
>
> --
> Catalin
>
Sergey
On Tue, 2009-07-14 at 17:03 +0300, Sergey Senozhatsky wrote:
> On (07/14/09 14:39), Catalin Marinas wrote:
> > On Tue, 2009-07-14 at 13:57 +0300, Sergey Senozhatsky wrote:
> > [...]
> > > +/*
> > > + * Printing of the objects hex dump to the seq file. The number on lines
> > > + * to be printed is limited to HEX_MAX_LINES to prevent seq file spamming.
> > > + * The actual number of printed bytes depends on HEX_ROW_SIZE.
> > > + * It must be called with the object->lock held.
> > > + */
> > [...]
> >
> > The patch looks fine. Could you please add a description and
> > Signed-off-by line?
> >
>
> Sure. During 30-40 minutes (sorry, I'm a bit busy now). OK?
There is no hurry, sometime in the next few weeks :-)
> Should I update Documentation/kmemeleak.txt either?
I don't think this is needed as it doesn't say much about the format of
the debug/kmemleak file (and that's pretty clear, no need to explain
what a hex dump means).
--
Catalin
On (07/14/09 15:17), Catalin Marinas wrote:
> There is no hurry, sometime in the next few weeks :-)
>
> > Should I update Documentation/kmemeleak.txt either?
>
> I don't think this is needed as it doesn't say much about the format of
> the debug/kmemleak file (and that's pretty clear, no need to explain
> what a hex dump means).
>
Fixed typo "The number on lines".
-----------------------------------------------------------------------------
kmemleak: Printing of the objects hex dump
Introducing printing of the objects hex dump to the seq file.
The number of lines to be printed is limited to HEX_MAX_LINES
to prevent seq file spamming. The actual number of printed
bytes is less than or equal to (HEX_MAX_LINES * HEX_ROW_SIZE).
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 5aabd41..f7b74ac 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -161,6 +161,15 @@ struct kmemleak_object {
/* flag set on newly allocated objects */
#define OBJECT_NEW (1 << 3)
+/* number of bytes to print per line; must be 16 or 32 */
+#define HEX_ROW_SIZE 16
+/* number of bytes to print at a time (1, 2, 4, 8) */
+#define HEX_GROUP_SIZE 1
+/* include ASCII after the hex output */
+#define HEX_ASCII 1
+/* max number of lines to be printed */
+#define HEX_MAX_LINES 2
+
/* the list of all allocated objects */
static LIST_HEAD(object_list);
/* the list of gray-colored objects (see color_gray comment below) */
@@ -254,6 +263,35 @@ static void kmemleak_disable(void);
kmemleak_disable(); \
} while (0)
+
+/*
+ * Printing of the objects hex dump to the seq file. The number of lines
+ * to be printed is limited to HEX_MAX_LINES to prevent seq file spamming.
+ * The actual number of printed bytes depends on HEX_ROW_SIZE.
+ * It must be called with the object->lock held.
+ */
+static void hex_dump_object(struct seq_file *seq,
+ struct kmemleak_object *object)
+{
+ const u8 *ptr = (const u8 *)object->pointer;
+ /* Limit the number of lines to HEX_MAX_LINES. */
+ int len = min(object->size, (size_t)(HEX_MAX_LINES * HEX_ROW_SIZE));
+ int i, remaining = len;
+ unsigned char linebuf[200];
+
+ seq_printf(seq, " hex dump (first %d bytes):\n", len);
+
+ for (i = 0; i < len; i += HEX_ROW_SIZE) {
+ int linelen = min(remaining, HEX_ROW_SIZE);
+ remaining -= HEX_ROW_SIZE;
+ hex_dump_to_buffer(ptr + i, linelen, HEX_ROW_SIZE,
+ HEX_GROUP_SIZE, linebuf,
+ sizeof(linebuf), HEX_ASCII);
+
+ seq_printf(seq, " %s\n", linebuf);
+ }
+}
+
/*
* Object colors, encoded with count and min_count:
* - white - orphan object, not enough references to it (count < min_count)
@@ -304,6 +342,9 @@ static void print_unreferenced(struct seq_file *seq,
object->pointer, object->size);
seq_printf(seq, " comm \"%s\", pid %d, jiffies %lu\n",
object->comm, object->pid, object->jiffies);
+
+ hex_dump_object(seq, object);
+
seq_printf(seq, " backtrace:\n");
for (i = 0; i < object->trace_len; i++) {