lkml.org 
[lkml]   [2008]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 1/5] kmemtrace: Core implementation.
On Fri, 11 Jul 2008 11:41:20 +0300
"Pekka Enberg" <penberg@cs.helsinki.fi> wrote:

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

Sure. I thought "type" would be too confusing.

Regarding the use of "kmalloc" and "kmem_cache", I'd have a few objections:
1. "kmalloc" is ambiguous and may mislead some people into thinking
that such a type is inappropriate for kfree().
2. "kmem_cache" seemed way too long. The tracing function calls are
already kinda long.

> > +
> > +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.

__packed__ doesn't hurt and will keep us from getting fried when doing
cross-platform tracing (e.g. trace on ARM, analyze on x86) in case the
ABI changes. It's also a reminder that it's best to keep the structure
packed tight and avoid alignment.

> > +
> > +/* End of ABI definition. */


\
 
 \ /
  Last update: 2008-07-11 22:07    [W:0.046 / U:0.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site