Skip to contentSkip to conversation

relay: Add buffer-only channels; useful for early logging.

PATCH In review Eduard - Gabriel Munteanu 3 participants 5 messages Jun 23, 2008 – Aug 6, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

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.

This is needed by kmemtrace to do tracing in early kernel code.

---
 Documentation/filesystems/relay.txt |   11 +++
 include/linux/relay.h               |    5 +
 kernel/relay.c                      |  170 +++++++++++++++++++++++++++++------
 3 files changed, 157 insertions(+), 29 deletions(-)
Documentation/filesystems/relay.txt +11 -0
index 094f2d2..b417f83 100644
@@ -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
index 6cd8c44..953fc05 100644
@@ -48,6 +48,7 @@ struct rchan_buf
 	size_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;
 
@@ -68,6 +69,7 @@ struct rchan
 	int 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 +171,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 +141 -29

233 lines not on this page · raw message

Signature · 1 line
1.5.5.4
maintainer
#1 lore raw
Quoted 6 lines · On Mon, 23 Jun 2008 15:31:04 +0300 Eduard - Gabriel Munteanu…

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.

This is needed by kmemtrace to do tracing in early kernel code.

This breaks on sparc64.

kernel/relay.c free_bufs: · line 703

Quoted 3 lines
kernel/relay.c +3 -0
+			err = smp_call_function_single(i,
+						       __relay_set_buf_dentry,
+						       &disp, 0, 1);

Because that ain't implemented.

There's a call in net/iucv/iucv.c, but that's s390-only.

There's a call in virt/kvm/kvm_main.c.

There's a call in kernel/time/tick-broadcast.c, so I assume that the intersection between CONFIG_GENERIC_CLOCKEVENTS_BROADCAST and non-smp_call_function_single() architectures is presently empty.

I guess all SMP-capable architectures should now implement this, please. It is presently defined on all architectures for CONFIG_SMP=n and it is declared in include/linux/smp.h.

#2 lore raw
Quoted 10 lines · On Wed, 2 Jul 2008 23:06:36 -0700 Andrew Morton <[email protected]>…

This breaks on sparc64.

Quoted 5 lines
(quoted diff) +2 -0
+			err = smp_call_function_single(i,
+

__relay_set_buf_dentry,

(quoted diff) +1 -0
+						       &disp, 0,

1);

Because that ain't implemented.

Quoted 7 lines

There's a call in net/iucv/iucv.c, but that's s390-only.

There's a call in virt/kvm/kvm_main.c.

There's a call in kernel/time/tick-broadcast.c, so I assume that the intersection between CONFIG_GENERIC_CLOCKEVENTS_BROADCAST and non-smp_call_function_single() architectures is presently empty.

Hi,

I'm not sure what I should do. Maybe disable relay_late_setup_files() on sparc64, with an empty inline?

Quoted 3 lines

I guess all SMP-capable architectures should now implement this, please. It is presently defined on all architectures for CONFIG_SMP=n and it is declared in include/linux/smp.h.

sparc64 seems to have smp_call_function_mask(). If we have the generic kernel/smp.c in linux-next or -mmotm, then this will define smp_call_function_single() to call smp_call_function_mask().

Is there anything I can do regarding this patch? Does it work since kernel/smp.c reappeared?

Eduard

maintainer
#3 lore raw
Quoted 17 lines · On Sat, 5 Jul 2008 07:14:01 +0300 Eduard - Gabriel Munteanu…
Quoted 14 lines · On Wed, 2 Jul 2008 23:06:36 -0700 Andrew Morton <[email protected]>…

This breaks on sparc64.

Quoted 5 lines
(quoted diff) +2 -0
+			err = smp_call_function_single(i,
+

__relay_set_buf_dentry,

(quoted diff) +1 -0
+						       &disp, 0,

1);

Because that ain't implemented.

There's a call in net/iucv/iucv.c, but that's s390-only.

There's a call in virt/kvm/kvm_main.c.

Quoted 7 lines
Quoted 3 lines

There's a call in kernel/time/tick-broadcast.c, so I assume that the intersection between CONFIG_GENERIC_CLOCKEVENTS_BROADCAST and non-smp_call_function_single() architectures is presently empty.

Hi,

I'm not sure what I should do.

Nothing, I'd suggest. I think that all architectures should implement smp_call_function_single(). You can have a shot at doing that if you like, but the implementations will need to be reviewed and merged by the relevant architectures maintainers. (That's why I snuck linux-arch into the cc list last time).

Quoted 7 lines

Maybe disable relay_late_setup_files()
on sparc64, with an empty inline?

Quoted 3 lines

I guess all SMP-capable architectures should now implement this, please. It is presently defined on all architectures for CONFIG_SMP=n and it is declared in include/linux/smp.h.

Quoted 6 lines

sparc64 seems to have smp_call_function_mask(). If we have the generic kernel/smp.c in linux-next or -mmotm, then this will define smp_call_function_single() to call smp_call_function_mask().

Is there anything I can do regarding this patch? Does it work since kernel/smp.c reappeared?

Nope:

y:/usr/src/25> grep USE_GENERIC_SMP_HELPERS arch/*/Kconfig

arch/alpha/Kconfig:     select USE_GENERIC_SMP_HELPERS
arch/arm/Kconfig:       select USE_GENERIC_SMP_HELPERS
arch/ia64/Kconfig:      select USE_GENERIC_SMP_HELPERS
arch/m32r/Kconfig:      select USE_GENERIC_SMP_HELPERS
arch/mips/Kconfig:      select USE_GENERIC_SMP_HELPERS
arch/parisc/Kconfig:    select USE_GENERIC_SMP_HELPERS
arch/powerpc/Kconfig:   select USE_GENERIC_SMP_HELPERS if SMP
arch/sh/Kconfig:        select USE_GENERIC_SMP_HELPERS
arch/x86/Kconfig:       select USE_GENERIC_SMP_HELPERS

It would help if you could check to see which architectures need work and then perhaps propose patches. If not, well, 2.6.27-rc1 will be broken on some architectures on some configs and people will have a couple of months to unbreak it. Not a big problem, I expect.

#4 lore raw
In reply to Eduard - Gabriel Munteanu

Hi,

I looks like this patch (commit 20d8b67c06fa5e74f44e80b0a0fd68c8327f7c6a) broke LTTng. LTTng expects its buffer file creation callback to be called when it calls relay_open. Actually, it passes a filename to the relay_open, so I don't see why the create_buf_file callback is not being called anymore. I'll revert it in my LTTng tree for now.

I'll eventually need a new create_buf() callback to correctly handle this and allow LTTng to do early boot tracing. When this new callback gets called, I can setup the data structures I use to manage my internal buffer offsets. Then the already existing create_buf_file callback could just do the debugfs calls.

Mathieu

Quoted 321 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.

This is needed by kmemtrace to do tracing in early kernel code.

---
 Documentation/filesystems/relay.txt |   11 +++
 include/linux/relay.h               |    5 +
 kernel/relay.c                      |  170 +++++++++++++++++++++++++++++------
 3 files changed, 157 insertions(+), 29 deletions(-)
Documentation/filesystems/relay.txt +11 -0
index 094f2d2..b417f83 100644
@@ -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
index 6cd8c44..953fc05 100644
@@ -48,6 +48,7 @@ struct rchan_buf
 	size_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;
 
@@ -68,6 +69,7 @@ struct rchan
 	int 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 +171,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 +141 -29
index 7de644c..4387b35 100644
@@ -407,6 +407,35 @@ void relay_reset(struct rchan *chan)
 }
 EXPORT_SYMBOL_GPL(relay_reset);
 
+static inline void relay_set_buf_dentry(struct rchan_buf *buf,
+					struct dentry *dentry)
+{
+	buf->dentry = dentry;
+	buf->dentry->d_inode->i_size = buf->early_bytes;
+}
+
+static struct dentry *relay_create_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)
+		return NULL;
+	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);
+
+	return dentry;
+}
+
 /*
  *	relay_open_buf - create a new relay channel buffer
  *
@@ -416,45 +445,34 @@ 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;
+
+	if (chan->has_base_filename) {
+		dentry = relay_create_buf_file(chan, buf, cpu);
+		if (!dentry)
+			goto free_buf;
+		relay_set_buf_dentry(buf, dentry);
+	}
 
  	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;
 }
 
 /**
@@ -537,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
@@ -560,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;
@@ -576,7 +592,10 @@ 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);
 
@@ -604,6 +623,94 @@ free_bufs:
 }
 EXPORT_SYMBOL_GPL(relay_open);
 
+struct rchan_percpu_buf_dispatcher {
+	struct rchan_buf *buf;
+	struct dentry *dentry;
+};
+
+/* Called in atomic context. */
+static void __relay_set_buf_dentry(void *info)
+{
+	struct rchan_percpu_buf_dispatcher *p = info;
+
+	relay_set_buf_dentry(p->buf, p->dentry);
+}
+
+/**
+ *	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)
+{
+	int err = 0;
+	unsigned int i, curr_cpu;
+	unsigned long flags;
+	struct dentry *dentry;
+	struct rchan_percpu_buf_dispatcher disp;
+
+	if (!chan || !base_filename)
+		return -EINVAL;
+
+	strlcpy(chan->base_filename, base_filename, NAME_MAX);
+
+	mutex_lock(&relay_channels_mutex);
+	/* Is chan already set up? */
+	if (unlikely(chan->has_base_filename))
+		return -EEXIST;
+	chan->has_base_filename = 1;
+	chan->parent = parent;
+	curr_cpu = get_cpu();
+	/*
+	 * The CPU hotplug notifier ran before us and created buffers with
+	 * no files associated. So it's safe to call relay_setup_buf_file()
+	 * on all currently online CPUs.
+	 */
+	for_each_online_cpu(i) {
+		if (unlikely(!chan->buf[i])) {
+			printk(KERN_ERR "relay_late_setup_files: CPU %u "
+					"has no buffer, it must have!\n", i);
+			BUG();
+			err = -EINVAL;
+			break;
+		}
+
+		dentry = relay_create_buf_file(chan, chan->buf[i], i);
+		if (unlikely(!dentry)) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (curr_cpu == i) {
+			local_irq_save(flags);
+			relay_set_buf_dentry(chan->buf[i], dentry);
+			local_irq_restore(flags);
+		} else {
+			disp.buf = chan->buf[i];
+			disp.dentry = dentry;
+			smp_mb();
+			/* relay_channels_mutex must be held, so wait. */
+			err = smp_call_function_single(i,
+						       __relay_set_buf_dentry,
+						       &disp, 0, 1);
+		}
+		if (unlikely(err))
+			break;
+	}
+	put_cpu();
+	mutex_unlock(&relay_channels_mutex);
+
+	return err;
+}
+
 /**
  *	relay_switch_subbuf - switch to a new sub-buffer
  *	@buf: channel buffer
@@ -627,8 +734,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))
 			/*
@@ -1237,4 +1349,4 @@ static __init int relay_init(void)
 	return 0;
 }
 
-module_init(relay_init);
+early_initcall(relay_init);
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