On Sun, Jul 03, 2016 at 09:45:27PM +0530, akash.g...@intel.com wrote: > From: Akash Goel <akash.g...@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.munte...@linux360.ro> > Cc: Tom Zanussi <tzanu...@gmail.com> > Cc: Chris Wilson <ch...@chris-wilson.co.uk> > Signed-off-by: Akash Goel <akash.g...@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