[PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

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

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Documentation/filesystems/relay.txt |   11 +++
 include/linux/relay.h               |    5 ++
 kernel/relay.c                      |  123 ++++++++++++++++++++++++++---------
 3 files changed, 107 insertions(+), 32 deletions(-)

diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
index 094f2d2..b417f83 100644
--- a/Documentation/filesystems/relay.txt
+++ b/Documentation/filesystems/relay.txt
@@ -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'
 ---------------
 
diff --git a/include/linux/relay.h b/include/linux/relay.h
index a3a03e7..1d3dcf8 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -49,6 +49,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;
 
@@ -69,6 +70,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 */
 };
 
@@ -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,
diff --git a/kernel/relay.c b/kernel/relay.c
index 1d2c3d1..7ac7a22 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -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))
 			/*
-- 
1.5.5.4

[PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

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

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Documentation/filesystems/relay.txt |   11 +++
 include/linux/relay.h               |    5 ++
 kernel/relay.c                      |  123 ++++++++++++++++++++++++++---------
 3 files changed, 107 insertions(+), 32 deletions(-)

diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
index 094f2d2..b417f83 100644
--- a/Documentation/filesystems/relay.txt
+++ b/Documentation/filesystems/relay.txt
@@ -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'
 ---------------
 
diff --git a/include/linux/relay.h b/include/linux/relay.h
index a3a03e7..1d3dcf8 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -49,6 +49,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;
 
@@ -69,6 +70,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 */
 };
 
@@ -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,
diff --git a/kernel/relay.c b/kernel/relay.c
index 250a27a..a544172 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -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))
 			/*
-- 
1.5.5.4

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Pekka Enberg]

Hi Eduard,

Eduard - Gabriel Munteanu wrote:
> @@ -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]);

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

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Eduard - Gabriel Munteanu]

On Fri, 13 Jun 2008 10:04:57 +0300
Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> 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?

Yes, that's true. Only one CPU is online at that early point.

> 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

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Eduard - Gabriel Munteanu]

Hi everybody,

Okay, let's not merge this yet. I'm working on getting CPU hotplug to
do its job early on.


	Cheers,
	Eduard

[PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

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

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Documentation/filesystems/relay.txt |   11 +++
 include/linux/relay.h               |    5 +
 kernel/relay.c                      |  141 +++++++++++++++++++++++++++--------
 3 files changed, 126 insertions(+), 31 deletions(-)

diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
index 094f2d2..b417f83 100644
--- a/Documentation/filesystems/relay.txt
+++ b/Documentation/filesystems/relay.txt
@@ -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'
 ---------------
 
diff --git a/include/linux/relay.h b/include/linux/relay.h
index a3a03e7..1d3dcf8 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -49,6 +49,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;
 
@@ -69,6 +70,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 */
 };
 
@@ -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,
diff --git a/kernel/relay.c b/kernel/relay.c
index 250a27a..80a6a40 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -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,33 @@ 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 +557,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 +580,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,7 +594,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);
 
@@ -607,6 +626,62 @@ 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 err;
+	struct rchan_buf *buf;
+
+	if (!chan || !base_filename)
+		return 1;
+
+	strlcpy(chan->base_filename, base_filename, NAME_MAX);
+
+	mutex_lock(&relay_channels_mutex);
+	if (unlikely(chan->has_base_filename))
+		goto out;
+	chan->has_base_filename = 1;
+	chan->parent = parent;
+	/*
+	 * 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) {
+		buf = chan->buf[i];
+
+		if (unlikely(!buf)) {
+			printk(KERN_ERR "relay_late_setup_files: "
+			       "all CPUs should have buffers!\n");
+			goto out;
+		}
+
+		err = relay_setup_buf_file(chan, buf, i);
+		if (unlikely(err))
+			goto out;
+	}
+	mutex_unlock(&relay_channels_mutex);
+
+	return 0;
+
+out:
+	mutex_unlock(&relay_channels_mutex);
+	return 1;
+}
+
+/**
  *	relay_switch_subbuf - switch to a new sub-buffer
  *	@buf: channel buffer
  *	@length: size of current event
@@ -629,8 +704,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))
 			/*
@@ -1241,9 +1321,8 @@ EXPORT_SYMBOL_GPL(relay_file_operations);
 
 static __init int relay_init(void)
 {
-
 	hotcpu_notifier(relay_hotcpu_callback, 0);
 	return 0;
 }
 
-module_init(relay_init);
+early_initcall(relay_init);
-- 
1.5.5.4

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Mathieu Desnoyers]

* Eduard - Gabriel Munteanu (eduard.munteanu@linux360.ro) wrote:
> On Fri, 13 Jun 2008 10:04:57 +0300
> Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> 
> > 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?
> 
> Yes, that's true. Only one CPU is online at that early point.
> 
> > 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.

> 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

> 
> 	Eduard
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Mathieu Desnoyers]

* Eduard - Gabriel Munteanu (eduard.munteanu@linux360.ro) 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

> Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> ---
>  Documentation/filesystems/relay.txt |   11 +++
>  include/linux/relay.h               |    5 ++
>  kernel/relay.c                      |  123 ++++++++++++++++++++++++++---------
>  3 files changed, 107 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
> index 094f2d2..b417f83 100644
> --- a/Documentation/filesystems/relay.txt
> +++ b/Documentation/filesystems/relay.txt
> @@ -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'
>  ---------------
>  
> diff --git a/include/linux/relay.h b/include/linux/relay.h
> index a3a03e7..1d3dcf8 100644
> --- a/include/linux/relay.h
> +++ b/include/linux/relay.h
> @@ -49,6 +49,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;
>  
> @@ -69,6 +70,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 */
>  };
>  
> @@ -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,
> diff --git a/kernel/relay.c b/kernel/relay.c
> index 250a27a..a544172 100644
> --- a/kernel/relay.c
> +++ b/kernel/relay.c
> @@ -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))
>  			/*
> -- 
> 1.5.5.4
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Eduard - Gabriel Munteanu]

On Mon, 16 Jun 2008 08:35:35 -0400
Mathieu Desnoyers <compudj@krystal.dyndns.org> wrote:
 
> 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

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Eduard - Gabriel Munteanu]

On Mon, 16 Jun 2008 08:30:03 -0400
Mathieu Desnoyers <compudj@krystal.dyndns.org> wrote:

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

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Mathieu Desnoyers]

* Eduard - Gabriel Munteanu (eduard.munteanu@linux360.ro) wrote:
> On Mon, 16 Jun 2008 08:30:03 -0400
> Mathieu Desnoyers <compudj@krystal.dyndns.org> wrote:
> 
> > 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.
>   
> > 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

> 
> 	Eduard
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Mathieu Desnoyers]

* Eduard - Gabriel Munteanu (eduard.munteanu@linux360.ro) 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.
> 
> Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> ---
>  Documentation/filesystems/relay.txt |   11 +++
>  include/linux/relay.h               |    5 +
>  kernel/relay.c                      |  141 +++++++++++++++++++++++++++--------
>  3 files changed, 126 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
> index 094f2d2..b417f83 100644
> --- a/Documentation/filesystems/relay.txt
> +++ b/Documentation/filesystems/relay.txt
> @@ -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'
>  ---------------
>  
> diff --git a/include/linux/relay.h b/include/linux/relay.h
> index a3a03e7..1d3dcf8 100644
> --- a/include/linux/relay.h
> +++ b/include/linux/relay.h
> @@ -49,6 +49,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;
>  
> @@ -69,6 +70,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 */
>  };
>  
> @@ -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,
> diff --git a/kernel/relay.c b/kernel/relay.c
> index 250a27a..80a6a40 100644
> --- a/kernel/relay.c
> +++ b/kernel/relay.c
> @@ -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);

spin_lock_* on a variable named rw_lock, insn't it a bit confusing ?
See rwlock_* and friends to figure out why this naming is not so
appropriate.

> +	buf->dentry = dentry;
> +	buf->dentry->d_inode->i_size = buf->early_bytes;
> +	spin_unlock_irqrestore(&buf->rw_lock, flags);

As long as this rw_lock is not taken on the read/write, etc path, it
won't protect against races. I think it should be left to the
relay_setup_buf_file() caller to provide proper synchronization, and
therefore begs for a comment on top of relay_late_setup_files().

Note that since relay_late_setup_files() takes a mutex, locking will
become a bit problematic, since you cannot nest a spinlock in a mutex.

> +
> +	return 0;
> +
> +failed:
> +	return 1;
> +}
> +
>  /*
>   *	relay_open_buf - create a new relay channel buffer
>   *
> @@ -415,48 +448,33 @@ 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 +557,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 +580,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,7 +594,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);
>  
> @@ -607,6 +626,62 @@ 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 err;
> +	struct rchan_buf *buf;
> +
> +	if (!chan || !base_filename)
> +		return 1;
> +
> +	strlcpy(chan->base_filename, base_filename, NAME_MAX);
> +
> +	mutex_lock(&relay_channels_mutex);
> +	if (unlikely(chan->has_base_filename))
> +		goto out;
> +	chan->has_base_filename = 1;
> +	chan->parent = parent;
> +	/*
> +	 * 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.

The following scenario will hide data :

cpu0 online
cpu1 online
tracing some stuff
cpu1 offline
relay_late_setup_files() is called.

You should iterate on all possible cpus and detect which ones have
buffers allocated.


> +	 */
> +	for_each_online_cpu(i) {
> +		buf = chan->buf[i];
> +
> +		if (unlikely(!buf)) {
> +			printk(KERN_ERR "relay_late_setup_files: "
> +			       "all CPUs should have buffers!\n");
> +			goto out;
> +		}
> +
> +		err = relay_setup_buf_file(chan, buf, i);
> +		if (unlikely(err))
> +			goto out;
> +	}
> +	mutex_unlock(&relay_channels_mutex);
> +
> +	return 0;
> +
> +out:
> +	mutex_unlock(&relay_channels_mutex);
> +	return 1;
> +}
> +
> +/**
>   *	relay_switch_subbuf - switch to a new sub-buffer
>   *	@buf: channel buffer
>   *	@length: size of current event
> @@ -629,8 +704,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))
>  			/*
> @@ -1241,9 +1321,8 @@ EXPORT_SYMBOL_GPL(relay_file_operations);
>  
>  static __init int relay_init(void)
>  {
> -

Whitespace removal does not belong in this patch.

>  	hotcpu_notifier(relay_hotcpu_callback, 0);
>  	return 0;
>  }
>  
> -module_init(relay_init);
> +early_initcall(relay_init);

See discussion about "how early" in the previous patch of this patchset.

> -- 
> 1.5.5.4
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

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

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Documentation/filesystems/relay.txt |   11 +++
 include/linux/relay.h               |    5 +
 kernel/relay.c                      |  141 +++++++++++++++++++++++++++--------
 3 files changed, 126 insertions(+), 31 deletions(-)

diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
index 094f2d2..b417f83 100644
--- a/Documentation/filesystems/relay.txt
+++ b/Documentation/filesystems/relay.txt
@@ -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'
 ---------------
 
diff --git a/include/linux/relay.h b/include/linux/relay.h
index a3a03e7..1d3dcf8 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -49,6 +49,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;
 
@@ -69,6 +70,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 */
 };
 
@@ -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,
diff --git a/kernel/relay.c b/kernel/relay.c
index 250a27a..80a6a40 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -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,33 @@ 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 +557,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 +580,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,7 +594,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);
 
@@ -607,6 +626,62 @@ 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 err;
+	struct rchan_buf *buf;
+
+	if (!chan || !base_filename)
+		return 1;
+
+	strlcpy(chan->base_filename, base_filename, NAME_MAX);
+
+	mutex_lock(&relay_channels_mutex);
+	if (unlikely(chan->has_base_filename))
+		goto out;
+	chan->has_base_filename = 1;
+	chan->parent = parent;
+	/*
+	 * 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) {
+		buf = chan->buf[i];
+
+		if (unlikely(!buf)) {
+			printk(KERN_ERR "relay_late_setup_files: "
+			       "all CPUs should have buffers!\n");
+			goto out;
+		}
+
+		err = relay_setup_buf_file(chan, buf, i);
+		if (unlikely(err))
+			goto out;
+	}
+	mutex_unlock(&relay_channels_mutex);
+
+	return 0;
+
+out:
+	mutex_unlock(&relay_channels_mutex);
+	return 1;
+}
+
+/**
  *	relay_switch_subbuf - switch to a new sub-buffer
  *	@buf: channel buffer
  *	@length: size of current event
@@ -629,8 +704,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))
 			/*
@@ -1241,9 +1321,8 @@ EXPORT_SYMBOL_GPL(relay_file_operations);
 
 static __init int relay_init(void)
 {
-
 	hotcpu_notifier(relay_hotcpu_callback, 0);
 	return 0;
 }
 
-module_init(relay_init);
+early_initcall(relay_init);
-- 
1.5.5.4

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Pekka Enberg]

On Sat, Jun 21, 2008 at 5:11 PM, Eduard - Gabriel Munteanu
<eduard.munteanu@linux360.ro> 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.
>
> Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Pekka Enberg]

On Sat, Jun 21, 2008 at 5:11 PM, Eduard - Gabriel Munteanu
<eduard.munteanu@linux360.ro> 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.
>
> +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;

Why don't you return -ENOMEM here?

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

And here?

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

The separate goto seems pointless. Why don't you use negative return
values for error handling?

> +}
> +
>  /*
>  *     relay_open_buf - create a new relay channel buffer
>  *
> @@ -415,48 +448,33 @@ 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 +557,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 +580,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,7 +594,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);
>
> @@ -607,6 +626,62 @@ 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 err;
> +       struct rchan_buf *buf;
> +
> +       if (!chan || !base_filename)
> +               return 1;

-EINVAL?

> +
> +       strlcpy(chan->base_filename, base_filename, NAME_MAX);
> +
> +       mutex_lock(&relay_channels_mutex);
> +       if (unlikely(chan->has_base_filename))
> +               goto out;

-EINVAL?

> +       chan->has_base_filename = 1;
> +       chan->parent = parent;
> +       /*
> +        * 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) {
> +               buf = chan->buf[i];
> +
> +               if (unlikely(!buf)) {
> +                       printk(KERN_ERR "relay_late_setup_files: "
> +                              "all CPUs should have buffers!\n");
> +                       goto out;
> +               }
> +
> +               err = relay_setup_buf_file(chan, buf, i);
> +               if (unlikely(err))
> +                       goto out;
> +       }
> +       mutex_unlock(&relay_channels_mutex);
> +
> +       return 0;
> +
> +out:
> +       mutex_unlock(&relay_channels_mutex);
> +       return 1;

You don't need two return statements (and coincidentally two unlocks)
if you keep the return value in a local variable 'err'.

> +}
> +
> +/**
>  *     relay_switch_subbuf - switch to a new sub-buffer
>  *     @buf: channel buffer
>  *     @length: size of current event
> @@ -629,8 +704,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))
>                        /*
> @@ -1241,9 +1321,8 @@ EXPORT_SYMBOL_GPL(relay_file_operations);
>
>  static __init int relay_init(void)
>  {
> -
>        hotcpu_notifier(relay_hotcpu_callback, 0);
>        return 0;
>  }
>
> -module_init(relay_init);
> +early_initcall(relay_init);
> --
> 1.5.5.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Eduard - Gabriel Munteanu]

On Sun, 22 Jun 2008 22:29:53 +0300
"Pekka Enberg" <penberg@cs.helsinki.fi> wrote:

> On Sat, Jun 21, 2008 at 5:11 PM, Eduard - Gabriel Munteanu
> <eduard.munteanu@linux360.ro> wrote:
> > +       tmpname = kzalloc(NAME_MAX + 1, GFP_KERNEL);
> > +       if (!tmpname)
> > +               goto failed;
> 
> Why don't you return -ENOMEM here?

The original function from which I ripped this thing off
(relay_open_buf()) returned 1 on error, so I assumed I wasn't supposed
to change this behavior.

I should probably do this in a separate patch.

> The separate goto seems pointless. Why don't you use negative return
> values for error handling?

Yes, it's better to use 'return' directly.

> >  /**
> > + *     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 err;
> > +       struct rchan_buf *buf;
> > +
> > +       if (!chan || !base_filename)
> > +               return 1;
> 
> -EINVAL?

Will do and resubmit. Being new code (called directly by other kernel
users), it allows me to be a bit inconsistent about return values.

> > +               err = relay_setup_buf_file(chan, buf, i);
> > +               if (unlikely(err))
> > +                       goto out;
> > +       }
> > +       mutex_unlock(&relay_channels_mutex);
> > +
> > +       return 0;
> > +
> > +out:
> > +       mutex_unlock(&relay_channels_mutex);
> > +       return 1;
> 
> You don't need two return statements (and coincidentally two unlocks)
> if you keep the return value in a local variable 'err'.

Thanks, I missed this possibility. Sounds better.
 

	Eduard

[PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

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

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Documentation/filesystems/relay.txt |   11 +++
 include/linux/relay.h               |    5 +
 kernel/relay.c                      |  170 +++++++++++++++++++++++++++++------
 3 files changed, 157 insertions(+), 29 deletions(-)

diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
index 094f2d2..b417f83 100644
--- a/Documentation/filesystems/relay.txt
+++ b/Documentation/filesystems/relay.txt
@@ -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'
 ---------------
 
diff --git a/include/linux/relay.h b/include/linux/relay.h
index 6cd8c44..953fc05 100644
--- a/include/linux/relay.h
+++ b/include/linux/relay.h
@@ -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,
diff --git a/kernel/relay.c b/kernel/relay.c
index 7de644c..4387b35 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -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);
-- 
1.5.5.4

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Andrew Morton]

On Mon, 23 Jun 2008 15:31:04 +0300 Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> 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.

This breaks on sparc64.

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

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Eduard - Gabriel Munteanu]

On Wed, 2 Jul 2008 23:06:36 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
 
> This breaks on sparc64.
> 
> > +			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.

Hi,

I'm not sure what I should do. Maybe disable relay_late_setup_files()
on sparc64, with an empty inline?
 
> 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

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Andrew Morton]

On Sat, 5 Jul 2008 07:14:01 +0300 Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:

> On Wed, 2 Jul 2008 23:06:36 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
>  
> > This breaks on sparc64.
> > 
> > > +			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.
> 
> 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).

> Maybe disable relay_late_setup_files()
> on sparc64, with an empty inline?
>  
> > 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?

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.

Re: [PATCH 3/3] relay: Add buffer-only channels; useful for early logging.

[Mathieu Desnoyers]

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

* Eduard - Gabriel Munteanu (eduard.munteanu@linux360.ro) 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.
> 
> Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> ---
>  Documentation/filesystems/relay.txt |   11 +++
>  include/linux/relay.h               |    5 +
>  kernel/relay.c                      |  170 +++++++++++++++++++++++++++++------
>  3 files changed, 157 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/filesystems/relay.txt b/Documentation/filesystems/relay.txt
> index 094f2d2..b417f83 100644
> --- a/Documentation/filesystems/relay.txt
> +++ b/Documentation/filesystems/relay.txt
> @@ -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'
>  ---------------
>  
> diff --git a/include/linux/relay.h b/include/linux/relay.h
> index 6cd8c44..953fc05 100644
> --- a/include/linux/relay.h
> +++ b/include/linux/relay.h
> @@ -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,
> diff --git a/kernel/relay.c b/kernel/relay.c
> index 7de644c..4387b35 100644
> --- a/kernel/relay.c
> +++ b/kernel/relay.c
> @@ -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);
> -- 
> 1.5.5.4
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68