lkml.org 
[lkml]   [2016]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] relay: Add global mode support for buffer-only channels
On Sun, Jul 03, 2016 at 09:45:27PM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
>
> 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@linux360.ro>
> Cc: Tom Zanussi <tzanussi@gmail.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
> 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 */

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

?

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

--
Chris Wilson, Intel Open Source Technology Centre

\
 
 \ /
  Last update: 2016-07-04 10:21    [W:0.056 / U:1.760 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site