relay: Add buffer-only channels; useful for early logging.
Subsystems (2)
PATCH 3/3 relay: Add buffer-only channels; useful for early logging.
Allows one to create and use a channel with no associated files. Files can be initialized later. This is useful in scenarios such as logging in early code, before VFS is up. Therefore, such channels can be created and used as soon as kmem_cache_init() completed.
--- Documentation/filesystems/relay.txt | 11 +++ include/linux/relay.h | 5 ++ kernel/relay.c | 123 ++++++++++++++++++++++++++--------- 3 files changed, 107 insertions(+), 32 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 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 +5 -0
@@ -49,6 +49,7 @@ struct rchan_bufsize_t *padding;/* padding counts per sub-buffer */ size_t prev_padding;/* temporary variable */ size_t bytes_consumed;/* bytes consumed in cur read subbuf */ +size_t early_bytes;/* bytes consumed before VFS inited */ unsigned int cpu;/* this buf's cpu */ }____cacheline_aligned ;
@@ -69,6 +70,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 */ };
@@ -170,6 +172,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 +91 -32
194 lines not on this page · raw message
Signature · 1 line
1.5.5.4
Hi Eduard,
Quoted 19 lines · Eduard - Gabriel Munteanu
kernel/relay.c +5
-2@@ -578,12 +592,15 @@ 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 ); + }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;
kernel/relay.c struct rchan *relay_open(const char *base_filename, · line 617
Quoted 8 lines
kernel/relay.c +1
-1@@ -594,7 +611,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]);
Why do we need to change for_each_online_cpu to for_each_present_cpu? I guess it's because we don't have all the CPUs online at early boot? Wouldn't it then be better to implement CPU hotplug support instead?
Pekka
Quoted 3 lines · On Fri, 13 Jun 2008 10:04:57 +0300 Pekka Enberg <[email protected]> wrote:
Why do we need to change
for_each_online_cputofor_each_present_cpu? I guess it's because we don't have all the CPUs online at early boot?
Yes, that's true. Only one CPU is online at that early point.
Quoted 2 lines
Wouldn't it then be better to implement CPU hotplug support instead?
It's difficult for me to write such code, as I'm unable to test CPU hotplug; my machines are a Turion X2 laptop and a Athlon Barton desktop computer. So I can't turn off, or unplug in a software sense, one CPU core. Correct me if I'm wrong, but I don't see any way to do that in /sys/devices/system/cpu/
But I suppose I could write a patch very carefully, and then submit to someone that can test this. Anyway, I'll look into this and try to understand the code without being able to test.
Eduard
Quoted 9 lines · * Eduard - Gabriel Munteanu ([email protected]) wrote:
Quoted 3 lines · On Fri, 13 Jun 2008 10:04:57 +0300 Pekka Enberg <[email protected]> wrote:
Why do we need to change
for_each_online_cputofor_each_present_cpu? I guess it's because we don't have all the CPUs online at early boot?Yes, that's true. Only one CPU is online at that early point.
Quoted 8 lines
Quoted 2 lines
Wouldn't it then be better to implement CPU hotplug support instead?
It's difficult for me to write such code, as I'm unable to test CPU hotplug; my machines are a Turion X2 laptop and a Athlon Barton desktop computer. So I can't turn off, or unplug in a software sense, one CPU core. Correct me if I'm wrong, but I don't see any way to do that in
/sys/devices/system/cpu/
See Documentation/cpu-hotplug.txt
If you need to "emulate" having many CPUs, Xen is always an option.
Quoted 4 lines
But I suppose I could write a patch very carefully, and then submit to someone that can test this. Anyway, I'll look into this and try to understand the code without being able to test.
Hrm, sounds like a bad idea... please try getting a test setup working, it will save you a gazillion iterations and let you stress-test your code.
Mathieu
Quoted 3 lines
Eduard
Signature · 2 lines
Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 1 line · On Mon, 16 Jun 2008 08:30:03 -0400 Mathieu Desnoyers…
If you need to "emulate" having many CPUs, Xen is always an option.
I'd rather set up a QEMU environment than rebase this on Xen.
Quoted 3 lines
Hrm, sounds like a bad idea... please try getting a test setup working, it will save you a gazillion iterations and let you stress-test your code.
Check your mailbox, I resubmitted the buffer-only channels along with hotplug fixes. I couldn't test on my machine all hotplug has to offer, but hotplug does create the buffers correctly when the kernel goes SMP.
Eduard
Quoted 7 lines · * Eduard - Gabriel Munteanu ([email protected]) wrote:
Quoted 1 line · On Mon, 16 Jun 2008 08:30:03 -0400 Mathieu Desnoyers…
If you need to "emulate" having many CPUs, Xen is always an option.
I'd rather set up a QEMU environment than rebase this on Xen.
Quoted 8 lines
Quoted 3 lines
Hrm, sounds like a bad idea... please try getting a test setup working, it will save you a gazillion iterations and let you stress-test your code.
Check your mailbox, I resubmitted the buffer-only channels along with hotplug fixes. I couldn't test on my machine all hotplug has to offer, but hotplug does create the buffers correctly when the kernel goes SMP.
testing cpu hotplug with online/offline cycling should be done too.
Mathieu
Quoted 3 lines
Eduard
Signature · 2 lines
Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Hi everybody,
Okay, let's not merge this yet. I'm working on getting CPU hotplug to do its job early on.
Cheers,
Eduard
Quoted 5 lines · * Eduard - Gabriel Munteanu ([email protected]) wrote:
Allows one to create and use a channel with no associated files. Files can be initialized later. This is useful in scenarios such as logging in early code, before VFS is up. Therefore, such channels can be created and used as soon as
kmem_cache_init()completed.
I think it would be good to add a patch to this patch series which allows to allocate such early boot buffers either statically in a kernel menuconfig option or in a kernel command line argument, so we do not depend on vmap. That would make it usable "very early".
Mathieu
Quoted 275 lines
Signed-off-by Eduard - Gabriel Munteanu <[email protected]>--- Documentation/filesystems/relay.txt | 11 +++ include/linux/relay.h | 5 ++ kernel/relay.c | 123 ++++++++++++++++++++++++++--------- 3 files changed, 107 insertions(+), 32 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 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 +5
-0@@ -49,6 +49,7 @@ struct rchan_bufsize_t *padding;/* padding counts per sub-buffer */ size_t prev_padding;/* temporary variable */ size_t bytes_consumed;/* bytes consumed in cur read subbuf */ +size_t early_bytes;/* bytes consumed before VFS inited */ unsigned int cpu;/* this buf's cpu */ }____cacheline_aligned ;@@ -69,6 +70,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 */ };@@ -170,6 +172,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 +91
-32@@ -407,6 +407,39 @@ 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; +unsigned long flags; +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); + +kfree (tmpname); + +if (!dentry) + goto failed; +spin_lock_irqsave (&buf->rw_lock, flags); + buf->dentry = dentry; + buf->dentry->d_inode->i_size = buf->early_bytes; +spin_unlock_irqrestore (&buf->rw_lock, flags); + +return 0 ; + +failed: +return 1 ; +} +/* * relay_open_buf - create a new relay channel buffer * @@ -415,48 +448,31 @@ 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; +return NULL ; -spin_lock_init (&buf->rw_lock); +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; +return buf; free_buf:relay_destroy_buf (buf); - buf =NULL ; -free_name: -kfree (tmpname); -end: -return buf; +return NULL ; }/** @@ -539,8 +555,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 @@ -562,8 +578,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 ;@@ -578,12 +592,15 @@ 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 ); + }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;@@ -594,7 +611,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]);@@ -607,6 +624,43 @@ 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 if 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; +int ret; +struct rchan_buf *buf; + +if (!chan || !base_filename) +return 1 ; + +strlcpy (chan->base_filename, base_filename,NAME_MAX ); + chan->has_base_filename =1 ; + chan->parent = parent; + +for_each_present_cpu (i) { + buf = chan->buf[i]; + + ret =relay_setup_buf_file (chan, buf, i); +if (unlikely (ret)) +return 1 ; + } + +return 0 ; +} + +/** * relay_switch_subbuf - switch to a new sub-buffer * @buf: channel buffer * @length: size of current event @@ -629,8 +683,13 @@ size_t relay_switch_subbuf(struct rchan_buf *buf, size_t length) old_subbuf = buf->subbufs_produced % buf->chan->n_subbufs; buf->padding[old_subbuf] = buf->prev_padding; buf->subbufs_produced++; - buf->dentry->d_inode->i_size += buf->chan->subbuf_size - - buf->padding[old_subbuf]; +if (buf->dentry) + buf->dentry->d_inode->i_size += + buf->chan->subbuf_size - + buf->padding[old_subbuf]; +else + buf->early_bytes += buf->chan->subbuf_size - + buf->padding[old_subbuf];smp_mb ();if (waitqueue_active (&buf->read_wait))/* Signature · 1 line
1.5.5.4
Signature · 2 lines
Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 6 lines · On Mon, 16 Jun 2008 08:35:35 -0400 Mathieu Desnoyers…
I think it would be good to add a patch to this patch series which allows to allocate such early boot buffers either statically in a kernel menuconfig option or in a kernel command line argument, so we do not depend on vmap. That would make it usable "very early".
Mathieu
This can be done, but it would require rewriting much of what relay uses to allocate structures and buffers. I've decided with my mentor, Pekka Enberg, that kmemtrace doesn't need this so much, so this is on my low priority list. I agree it would be nice, but it probably won't happen until kmemtrace is done.
The bright side is most of this patch will still be valid if we choose to implement static buffers. So this _is_ a step in that direction.
Eduard