On Sun, Jul 03, 2016 at 09:45:27PM +0530, akash.goel@xxxxxxxxx wrote:
From: Akash Goel <akash.goel@xxxxxxxxx>
The following patch added support to use channels with no associated files.
relay: add buffer-only channels; useful for early logging
This is useful when the exact location of relay file is not known or the
the parent directory of relay file is not available, while creating the
channel and the logging has to start right from the boot.
But there was no provision to use global mode with buffer-only channels,
which is added by this patch, without modifying the interface where initially
there will be a dummy invocation of create_buf_file callback through which
kernel client can convey the need of a global buffer.
For the use case where drivers/kernel clients want a simple interface for the
userspace, which enables them to capture data/logs from relay file in order &
without any post processing, support of Global buffer mode is warranted.
Cc: Eduard - Gabriel Munteanu <eduard.munteanu@xxxxxxxxxxx>
Cc: Tom Zanussi <tzanussi@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
---
kernel/relay.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/kernel/relay.c b/kernel/relay.c
index 04d7cf3..b267384 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -451,6 +451,16 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
if (!dentry)
goto free_buf;
relay_set_buf_dentry(buf, dentry);
+ } else {
The trailing block can be replaced with:
/* Only retrieve global info, nothing more, nothing less */
Thanks will combine the WARN and if check.+ dentry = chan->cb->create_buf_file(NULL, NULL,
+ S_IRUSR, buf,
+ &chan->is_global);
Watch alignment, align to the opening bracket.
+ if (WARN_ON(dentry))
+ goto free_buf;
}
buf->cpu = cpu;
@@ -666,6 +676,27 @@ int relay_late_setup_files(struct rchan *chan,
}
chan->has_base_filename = 1;
chan->parent = parent;
+
+ if (chan->is_global) {
+ if (unlikely(!chan->buf[0])) {
if (WARN_ON_ONCE(!chan->buf[0])) {
+ WARN_ONCE(1, KERN_ERR "CPU 0 has no buffer!\n");
The WARN includes the unlikely and the message here isn't any more
informative than the line itself, plus has the erroneous KERN_ERR
+ mutex_unlock(&relay_channels_mutex);
+ return -EINVAL;
+ }
+
+ dentry = relay_create_buf_file(chan, chan->buf[0], 0);
+
+ if (unlikely(!dentry))
+ err = -EINVAL;
+ else if (WARN_ON(!chan->is_global))
?
Thanks this is much better.+ err = -EINVAL;
+ else
+ relay_set_buf_dentry(chan->buf[0], dentry);
+
+ mutex_unlock(&relay_channels_mutex);
+ return err;
This could be written:
if (chan->is_global) {
err = -EINVAL;
if (!WARN_ON_ONCE(!chan->buf[0])) {
dentry = relay_create_buf_file(chan, chan->buf[0], 0))
if (dentry) {
relay_set_buf_dentry(chan->buf[0], dentry);
err = 0;
}
}
mutex_unlock();
return err;
}
for a single exit/unlock path in this block.
-Chris