Re: [RFC PATCH 1/5] kmemtrace: Core implementation.

[Pekka Enberg]

Hi Eduard-Gabriel,

On Thu, Jul 10, 2008 at 9:05 PM, Eduard - Gabriel Munteanu
<eduard.munteanu@xxxxxxxxxxx> 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 <eduard.munteanu@xxxxxxxxxxx>

> new file mode 100644
> index 0000000..11cd8e2
> --- /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_kind_id {
> +       KMEMTRACE_KIND_KERNEL = 0,      /* kmalloc() / kfree(). */
> +       KMEMTRACE_KIND_CACHE,           /* kmem_cache_*(). */
> +       KMEMTRACE_KIND_PAGES,           /* __get_free_pages() and friends. */
> +};

Can we do s/kind/type/, please? Also, the names "kernel" and "cache"
are confusing. Can we just call them "kmalloc" and "kmem_cache"
instead?

> +
> +struct kmemtrace_event {
> +       __u16           event_id;       /* Allocate or free? */
> +       __u16           kind_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__));

Why do you need to use the __packed__ attribute here? Looks like the
struct is already laid out properly.

> +
> +/* End of ABI definition. */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/