relay: add buffer-only functionality, allowing for early kernel tracing
Subsystems (2)
PATCH 1/1 relay: add buffer-only functionality, allowing for early kernel tracing
relay_open() can now handle NULL base_filename, to register a buffer-only channel. Using a new function, relay_late_setup_files(), one can assign files after creating the channel, thus allowing for doing early tracing in the kernel, before VFS is up.
--- Documentation/filesystems/relay.txt | 11 ++++ include/linux/relay.h | 4 ++ kernel/relay.c | 102 ++++++++++++++++++++++++++--------- 3 files changed, 91 insertions(+), 26 deletions(-)
Documentation/filesystems/relay.txt +11 -0
@@ -161,6 +161,7 @@ TBD(curr. line MT:/API/) relay_close(chan) relay_flush(chan) relay_reset(chan) + relay_late_setup_files(chan, base_filename, parent) channel management typically called on instigation of userspace:
@@ -294,6 +295,16 @@ user-defined data with a channel, and is immediately available (including in create_buf_file()) via chan->private_data or buf->chan->private_data. +Buffer-only channels +-------------------- + +These channels have no files associated and can be created with +relay_open(NULL, NULL, ...). Such channels are useful in scenarios such +as when doing early tracing in the kernel, before the VFS is up. In these +cases, one may open a channel a buffer-only channel and then call +relay_late_setup_files() when the kernel is ready to handle files, to expose +the buffered data to the userspace. + Channel 'modes' ---------------
include/linux/relay.h +4 -0
@@ -68,6 +68,7 @@ struct rchanint is_global;/* One global buffer ? */ struct list_head list;/* for channel list */ struct dentry *parent;/* parent dentry passed to open */ +int has_base_filename;/* has a filename associated? */ char base_filename[NAME_MAX ];/* saved base filename */ };
@@ -169,6 +170,9 @@ struct rchan *relay_open(const char *base_filename,size_t n_subbufs, struct rchan_callbacks *cb, void *private_data); +extern int relay_late_setup_files (struct rchan *chan, +const char *base_filename, +struct dentry *parent);extern void relay_close (struct rchan *chan);extern void relay_flush (struct rchan *chan); extern void relay_subbufs_consumed(struct rchan *chan,
kernel/relay.c +76 -26
167 lines not on this page · raw message
Signature · 1 line
1.5.2.5
Quoted 5 lines · * Eduard - Gabriel Munteanu ([email protected]) wrote:
relay_open()can now handle NULLbase_filename, to register a buffer-only channel. Using a new function,relay_late_setup_files(), one can assign files after creating the channel, thus allowing for doing early tracing in the kernel, before VFS is up.
I like this :) But could we trace event sooner if we did not depend on vmalloc to allocate the buffers ?
I am thinking of receiving a kernel parameter that would reserve a buffer in the linear kernel mapping (kmalloc ?), or to compile-in a static buffer, which could be used by relay instead of the vmalloc'd buffer. That would permit to trace the memory allocator and some other very early stuff. Basically, relay would have it's "memory pool" to manage and would do its allocations from this fixed buffer. No free would be required.
Mathieu
Quoted 239 lines
Signed-off-by Eduard - Gabriel Munteanu <[email protected]>--- Documentation/filesystems/relay.txt | 11 ++++ include/linux/relay.h | 4 ++ kernel/relay.c | 102 ++++++++++++++++++++++++++--------- 3 files changed, 91 insertions(+), 26 deletions(-)Documentation/filesystems/relay.txt +11
-0@@ -161,6 +161,7 @@ TBD(curr. line MT:/API/) relay_close(chan) relay_flush(chan) relay_reset(chan) + relay_late_setup_files(chan, base_filename, parent) channel management typically called on instigation of userspace:@@ -294,6 +295,16 @@ user-defined data with a channel, and is immediately available (including in create_buf_file()) via chan->private_data or buf->chan->private_data. +Buffer-only channels +-------------------- + +These channels have no files associated and can be created with +relay_open(NULL, NULL, ...). Such channels are useful in scenarios such +as when doing early tracing in the kernel, before the VFS is up. In these +cases, one may open a channel a buffer-only channel and then call +relay_late_setup_files() when the kernel is ready to handle files, to expose +the buffered data to the userspace. + Channel 'modes' ---------------include/linux/relay.h +4
-0@@ -68,6 +68,7 @@ struct rchanint is_global;/* One global buffer ? */ struct list_head list;/* for channel list */ struct dentry *parent;/* parent dentry passed to open */ +int has_base_filename;/* has a filename associated? */ char base_filename[NAME_MAX ];/* saved base filename */ };@@ -169,6 +170,9 @@ struct rchan *relay_open(const char *base_filename,size_t n_subbufs, struct rchan_callbacks *cb, void *private_data); +extern int relay_late_setup_files (struct rchan *chan, +const char *base_filename, +struct dentry *parent);extern void relay_close (struct rchan *chan);extern void relay_flush (struct rchan *chan); extern void relay_subbufs_consumed(struct rchan *chan,kernel/relay.c +76
-26@@ -378,6 +378,35 @@ void relay_reset(struct rchan *chan) }EXPORT_SYMBOL_GPL (relay_reset); +static int relay_setup_buf_file (struct rchan *chan, +struct rchan_buf *buf, +unsigned int cpu) +{ +struct dentry *dentry; +char *tmpname; + + tmpname =kzalloc (NAME_MAX +1 ,GFP_KERNEL ); +if (!tmpname) + goto failed; +snprintf (tmpname,NAME_MAX ,"%s%d" , chan->base_filename, cpu); + +/* Create file in fs */ + dentry = chan->cb->create_buf_file (tmpname, chan->parent, +S_IRUSR , buf, + &chan->is_global); + +if (!dentry) + goto failed; + buf->dentry = dentry; + +kfree (tmpname); + +return 0 ; + +failed: +return 1 ; +} +/* * relay_open_buf - create a new relay channel buffer * @@ -386,44 +415,30 @@ EXPORT_SYMBOL_GPL(relay_reset);static struct rchan_buf *relay_open_buf (struct rchan *chan,unsigned int cpu) {struct rchan_buf *buf =NULL ; -struct dentry *dentry; -char *tmpname;if (chan->is_global)return chan->buf[0 ]; - tmpname =kzalloc (NAME_MAX +1 ,GFP_KERNEL ); -if (!tmpname) - goto end; -snprintf (tmpname,NAME_MAX ,"%s%d" , chan->base_filename, cpu); - buf =relay_create_buf (chan);if (!buf) - goto free_name; + goto end; + +if (chan->has_base_filename) +if (relay_setup_buf_file (chan, buf, cpu)) goto free_buf; buf->cpu = cpu;__relay_reset (buf,1 ); -/* Create file in fs */ - dentry = chan->cb->create_buf_file (tmpname, chan->parent,S_IRUSR , - buf, &chan->is_global); -if (!dentry) - goto free_buf; - - buf->dentry = dentry; -if (chan->is_global) { chan->buf[0 ] = buf; buf->cpu =0 ; } - goto free_name; + goto end; free_buf:relay_destroy_buf (buf); buf =NULL ; -free_name: -kfree (tmpname); end:return buf; }@@ -508,8 +523,8 @@ static int __cpuinit relay_hotcpu_callback(struct notifier_block *nb,/** * relay_open - create a new relay channel -* @base_filename: base name of files to create -* @parent: dentry of parent directory, %NULL for root directory +* @base_filename: base name of files to create, %NULL for buffering only +* @parent: dentry of parent directory, %NULL for root directory or buffer * @subbuf_size: size of sub-buffers * @n_subbufs: number of sub-buffers * @cb: client callback functions @@ -531,8 +546,6 @@ struct rchan *relay_open(const char *base_filename, {unsigned int i;struct rchan *chan; -if (!base_filename) -return NULL ;if (!(subbuf_size && n_subbufs))return NULL ;@@ -547,12 +560,17 @@ struct rchan *relay_open(const char *base_filename, chan->alloc_size =FIX_SIZE (subbuf_size * n_subbufs); chan->parent = parent; chan->private_data = private_data; -strlcpy (chan->base_filename, base_filename,NAME_MAX ); +if (base_filename) { + chan->has_base_filename =1 ; +strlcpy (chan->base_filename, base_filename,NAME_MAX ); + }else { + chan->has_base_filename =0 ; + }setup_callbacks (chan, cb);kref_init (&chan->kref);mutex_lock (&relay_channels_mutex); -for_each_online_cpu (i) { +for_each_present_cpu (i) { chan->buf[i] =relay_open_buf (chan, i);if (!chan->buf[i]) goto free_bufs;@@ -563,7 +581,7 @@ struct rchan *relay_open(const char *base_filename,return chan; free_bufs: -for_each_online_cpu (i) { +for_each_present_cpu (i) {if (!chan->buf[i])break ;relay_close_buf (chan->buf[i]);@@ -576,6 +594,38 @@ free_bufs:EXPORT_SYMBOL_GPL (relay_open);/** +* relay_late_setup_files - triggers file creation +* @chan: channel to operate on +* @base_filename: base name of files to create +* @parent: dentry of parent directory, %NULL for root directory +* +* Returns 0 is successful, non-zero otherwise. +* +* Use to setup files for a previously buffer-only channel. +* Useful to do early tracing in kernel, before VFS is up, for example. +*/ +int relay_late_setup_files (struct rchan *chan, +const char *base_filename, +struct dentry *parent) +{ +unsigned int i; + +if (!chan || !base_filename) +return 1 ; + +strlcpy (chan->base_filename, base_filename,NAME_MAX ); + chan->has_base_filename =1 ; + chan->parent = parent; + +mutex_lock (&relay_channels_mutex); +for_each_present_cpu (i) +relay_setup_buf_file (chan, chan->buf[i], i); +mutex_unlock (&relay_channels_mutex); + +return 0 ; +} + +/** * relay_switch_subbuf - switch to a new sub-buffer * @buf: channel buffer * @length: size of current event Signature · 1 line
1.5.2.5
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 11 lines · On Fri, 4 Apr 2008 12:06:23 -0400 Mathieu Desnoyers…
I like this :) But could we trace event sooner if we did not depend on vmalloc to allocate the buffers ?
I am thinking of receiving a kernel parameter that would reserve a buffer in the linear kernel mapping (kmalloc ?), or to compile-in a static buffer, which could be used by relay instead of the vmalloc'd buffer. That would permit to trace the memory allocator and some other very early stuff. Basically, relay would have it's "memory pool" to manage and would do its allocations from this fixed buffer. No free would be required.
Quoted 1 line
Mathieu
Sure. I'm doing this as part of my GSoC application (well, I haven't been accepted yet). I had also proposed this to Pekka Enberg, my mentor, but he was reluctant to hack the relay code more invasively. Now that you agree with this, I'll work on it.
kmalloc() doesn't work before kmem_cache_init() and relay code uses kmalloc(), not vmalloc(), IIRC.
I would go another way. Have the relay client call __get_free_pages() or similar function, allocate the buffer and pass it on to a variant of relay_open(), let's name it relay_early_open(). __get_free_pages() can be used before kmem_cache_init() as far as I know. No free is required, since early tracing code implies it's built into the kernel and not as a module. What do you say?
Will you merge this or wait until I finish the very early tracing stuff?
Cheers,
Eduard
Quoted 16 lines · * Eduard - Gabriel Munteanu ([email protected]) wrote:
Quoted 11 lines · On Fri, 4 Apr 2008 12:06:23 -0400 Mathieu Desnoyers…
I like this :) But could we trace event sooner if we did not depend on vmalloc to allocate the buffers ?
I am thinking of receiving a kernel parameter that would reserve a buffer in the linear kernel mapping (kmalloc ?), or to compile-in a static buffer, which could be used by relay instead of the vmalloc'd buffer. That would permit to trace the memory allocator and some other very early stuff. Basically, relay would have it's "memory pool" to manage and would do its allocations from this fixed buffer. No free would be required.
Quoted 1 line
Mathieu
Quoted 8 lines
Sure. I'm doing this as part of my GSoC application (well, I haven't been accepted yet). I had also proposed this to Pekka Enberg, my mentor, but he was reluctant to hack the relay code more invasively. Now that you agree with this, I'll work on it.
kmalloc()doesn't work beforekmem_cache_init()and relay code useskmalloc(), notvmalloc(), IIRC.
It currently uses both :
It uses alloc_page and vmap to allocate the buffers and it uses kzalloc/kmalloc to allocate the relay structures.
Quoted 7 lines
I would go another way. Have the relay client call
__get_free_pages()or similar function, allocate the buffer and pass it on to a variant ofrelay_open(), let's name itrelay_early_open().__get_free_pages()can be used beforekmem_cache_init()as far as I know. No free is required, since early tracing code implies it's built into the kernel and not as a module. What do you say?
Then I guess we would depend on page_alloc_init(). It's not that bad, but I guess the question is : do we prefer to let users specify the buffer size to reserve for tracing on the kernel command line (that would require __get_free_pages()) or do we compile-in a static buffer size, which can be specified in the kernel configuration. I think providing both could be an option : flexibility _and_ *very* early tracing.
So making this relay interface flexible enough to receive a buffer either reserved by __get_free_pages() or a static buffer seems like a good compromise. It would also have to receive the buffer size from the caller.
Quoted 2 lines
Will you merge this or wait until I finish the very early tracing stuff?
As far as I am concerned, I can only merge this in the LTTng project to try it. I've added Tom Zanussi, the author of relay, who might have some ideas to share with us. I would wait until we have a more precise idea of the API before I modify LTTng to start using it.
Regards,
Mathieu
Quoted 3 lines
Cheers,
Eduard
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 7 lines · On Fri, 4 Apr 2008 12:40:22 -0400 Mathieu Desnoyers…
Then I guess we would depend on
page_alloc_init(). It's not that bad, but I guess the question is : do we prefer to let users specify the buffer size to reserve for tracing on the kernel command line (that would require__get_free_pages()) or do we compile-in a static buffer size, which can be specified in the kernel configuration. I think providing both could be an option : flexibility _and_ *very* early tracing.
I wasn't referring to the kernel's users specifying the buffer on the command line. For example, LTTng code (I'm not familiar with it) could allocate a buffer, by itself or using a helper function, and pass it to relay_early_open(). Of course, it could read the command line and call that helper function, passing the size required by the user as an argument.
Basically, I'm saying that having relay code manage a large buffer, allocating memory from it for each of its callers, could be very complex. It would be easier and less complex to let each tracing subsystem allocate memory on its own, either through a static buffer or with __get_free_pages().
Quoted 4 lines
So making this relay interface flexible enough to receive a buffer either reserved by
__get_free_pages()or a static buffer seems like a good compromise. It would also have to receive the buffer size from the caller.
Yes, it shouldn't matter to the relay interface whether the buffer was statically or dynamically allocated. All it needs to know is where it starts and how long it is.
BTW, I'll resubmit a modified patch soon, as suggested by Randy Dunlap.
Eduard
Quoted 8 lines · On Fri, 4 Apr 2008 12:40:22 -0400 Mathieu Desnoyers…
As far as I am concerned, I can only merge this in the LTTng project to try it. I've added Tom Zanussi, the author of relay, who might have some ideas to share with us. I would wait until we have a more precise idea of the API before I modify LTTng to start using it.
Regards,
Mathieu
Hi,
I've tested my patch a bit more and ran into a problem, and fixed it. Forget the old patch, it will panic the kernel if it has to cross sub-buffer boundaries. I'll polish the new one a bit (maybe update the docs) and resubmit, along with a piece of code from my GSoC project that uses the early relay interface (may provide an easy to try demo).
Unfortunately, as for doing earlier than kmem_cache_init() tracing, I don't have the time right now, as I have to carry on with my GSoC project. I looked through the source more thoroughly and the needed changes are not trivial at all. I'm not saying I won't work on it, but it won't be a priority.
By the way, I got a "delivery failure" message... is something wrong with Tom Zanussi's e-mail address? Google showed 2007 LKML entries with his e-mail address as <zanussi at comcast dot net>. In any case, when I'll resubmit the patch, I'll add this other address in the Cc list. Or should we reach him another way?
I'd like to get this merged as it is so far, and mailing it directly to akpm (or worse, Linus) doesn't sound like a good idea.
Cheers,
Eduard
Quoted 25 lines · On Fri, 4 Apr 2008 18:33:03 +0300 Eduard - Gabriel Munteanu wrote:
relay_open()can now handle NULLbase_filename, to register a buffer-only channel. Using a new function,relay_late_setup_files(), one can assign files after creating the channel, thus allowing for doing early tracing in the kernel, before VFS is up.Signed-off-by Eduard - Gabriel Munteanu <[email protected]>--- Documentation/filesystems/relay.txt | 11 ++++ include/linux/relay.h | 4 ++ kernel/relay.c | 102 ++++++++++++++++++++++++++--------- 3 files changed, 91 insertions(+), 26 deletions(-)Documentation/filesystems/relay.txt +1
-0@@ -161,6 +161,7 @@ TBD(curr. line MT:/API/) relay_close(chan) relay_flush(chan) relay_reset(chan) + relay_late_setup_files(chan, base_filename, parent) channel management typically called on instigation of userspace:
Documentation/filesystems/relay.txt user-defined data with a channel, and is immediately available · line 304
Quoted 10 lines
Documentation/filesystems/relay.txt +7
-0@@ -294,6 +295,16 @@ user-defined data with a channel, and is immediately available (including in create_buf_file()) via chan->private_data or buf->chan->private_data. +Buffer-only channels +-------------------- + +These channels have no files associated and can be created with +relay_open(NULL, NULL, ...). Such channels are useful in scenarios such +as when doing early tracing in the kernel, before the VFS is up. In these +cases, one may open a channel a buffer-only channel and then call
~~~~~~~~~
d/a channel/
Quoted 23 lines
Documentation/filesystems/relay.txt +3
-0+relay_late_setup_files() when the kernel is ready to handle files, to expose +the buffered data to the userspace. + Channel 'modes' ---------------kernel/relay.c +9
-0@@ -378,6 +378,35 @@ void relay_reset(struct rchan *chan) }EXPORT_SYMBOL_GPL (relay_reset); +static int relay_setup_buf_file (struct rchan *chan, +struct rchan_buf *buf, +unsigned int cpu) +{ +struct dentry *dentry; +char *tmpname; + + tmpname =kzalloc (NAME_MAX +1 ,GFP_KERNEL ); +if (!tmpname)
kernel/relay.c void relay_reset(struct rchan *chan) · line 399
Quoted 10 lines
kernel/relay.c +10
-0+ goto failed; +snprintf (tmpname,NAME_MAX ,"%s%d" , chan->base_filename, cpu); + +/* Create file in fs */ + dentry = chan->cb->create_buf_file (tmpname, chan->parent, +S_IRUSR , buf, + &chan->is_global); + +if (!dentry) + goto failed;
above leaks tmpname ?
Quoted 25 lines
kernel/relay.c +10
-4+ buf->dentry = dentry; + +kfree (tmpname); + +return 0 ; + +failed: +return 1 ; +} +/* * relay_open_buf - create a new relay channel buffer * @@ -386,44 +415,30 @@ EXPORT_SYMBOL_GPL(relay_reset);static struct rchan_buf *relay_open_buf (struct rchan *chan,unsigned int cpu) {struct rchan_buf *buf =NULL ; -struct dentry *dentry; -char *tmpname;if (chan->is_global)return chan->buf[0 ]; - tmpname =kzalloc (NAME_MAX +1 ,GFP_KERNEL ); -if (!tmpname)
kernel/relay.c EXPORT_SYMBOL_GPL(relay_reset); · line 427
Quoted 10 lines
kernel/relay.c +4
-4- goto end; -snprintf (tmpname,NAME_MAX ,"%s%d" , chan->base_filename, cpu); - buf =relay_create_buf (chan);if (!buf) - goto free_name; + goto end; + +if (chan->has_base_filename) +if (relay_setup_buf_file (chan, buf, cpu)) goto free_buf;
Put 'goto free_buf;' on separate line.
Quoted 29 lines
kernel/relay.c +1
-11buf->cpu = cpu;__relay_reset (buf,1 ); -/* Create file in fs */ - dentry = chan->cb->create_buf_file (tmpname, chan->parent,S_IRUSR , - buf, &chan->is_global); -if (!dentry) - goto free_buf; - - buf->dentry = dentry; -if (chan->is_global) { chan->buf[0 ] = buf; buf->cpu =0 ; } - goto free_name; + goto end; free_buf:relay_destroy_buf (buf); buf =NULL ; -free_name: -kfree (tmpname); end:return buf; }
kernel/relay.c free_bufs: · line 602
Quoted 9 lines
kernel/relay.c +6
-0@@ -576,6 +594,38 @@ free_bufs:EXPORT_SYMBOL_GPL (relay_open);/** +* relay_late_setup_files - triggers file creation +* @chan: channel to operate on +* @base_filename: base name of files to create +* @parent: dentry of parent directory, %NULL for root directory +* +* Returns 0 is successful, non-zero otherwise.
s/is/if/
Quoted 19 lines
kernel/relay.c +19
-0+* +* Use to setup files for a previously buffer-only channel. +* Useful to do early tracing in kernel, before VFS is up, for example. +*/ +int relay_late_setup_files (struct rchan *chan, +const char *base_filename, +struct dentry *parent) +{ +unsigned int i; + +if (!chan || !base_filename) +return 1 ; + +strlcpy (chan->base_filename, base_filename,NAME_MAX ); + chan->has_base_filename =1 ; + chan->parent = parent; + +mutex_lock (&relay_channels_mutex); +for_each_present_cpu (i)
kernel/relay.c free_bufs: · line 631
Quoted 10 lines
kernel/relay.c +7
-0+relay_setup_buf_file (chan, chan->buf[i], i); +mutex_unlock (&relay_channels_mutex); + +return 0 ; +} + +/** * relay_switch_subbuf - switch to a new sub-buffer * @buf: channel buffer * @length: size of current event
---
~Randy
Hi,
Thanks for spotting these problems, Randy. I'll fix them and resubmit.
Cheers,
Eduard