Hi Masami, On Tue, May 13, 2025 at 5:58 PM Masami Hiramatsu <[email protected]> wrote: > > Hi Jason, > > On Mon, 12 May 2025 10:49:32 +0800 > Jason Xing <[email protected]> wrote: > > > From: Jason Xing <[email protected]> > > > > In this version, only support dumping the counter for buffer full and > > implement the framework of how it works. Users MUST pass a valid @buf > > with a valid @len that is required to be larger than RELAY_DUMP_BUF_MAX_LEN > > to acquire which information indicated by @flags to dump. > > > > RELAY_DUMP_BUF_MAX_LEN shows the maximum len of the buffer if users > > choose to dump all the values. > > > > Users can use this buffer to do whatever they expect in their own kernel > > module, say, print to console/dmesg or write them into the relay buffer. > > > > Reviewed-by: Yushan Zhou <[email protected]> > > Signed-off-by: Jason Xing <[email protected]> > > --- > > include/linux/relay.h | 10 ++++++++++ > > kernel/relay.c | 35 +++++++++++++++++++++++++++++++++++ > > 2 files changed, 45 insertions(+) > > > > diff --git a/include/linux/relay.h b/include/linux/relay.h > > index 022cf11e5a92..7a442c4cbead 100644 > > --- a/include/linux/relay.h > > +++ b/include/linux/relay.h > > @@ -31,6 +31,15 @@ > > /* > > * Relay buffer error statistics dump > > */ > > +enum { > > + RELAY_DUMP_BUF_FULL = (1 << 0), > > + > > + RELAY_DUMP_LAST = RELAY_DUMP_BUF_FULL, > > + RELAY_DUMP_MASK = (RELAY_DUMP_LAST - 1) | RELAY_DUMP_LAST > > +}; > > + > > +#define RELAY_DUMP_BUF_MAX_LEN 32 > > + > > struct rchan_buf_error_stats > > { > > unsigned int full; /* counter for buffer full */ > > @@ -170,6 +179,7 @@ extern int relay_late_setup_files(struct rchan *chan, > > struct dentry *parent); > > extern void relay_close(struct rchan *chan); > > extern void relay_flush(struct rchan *chan); > > +extern void relay_dump(struct rchan *chan, char *buf, int len, int flags); > > extern void relay_subbufs_consumed(struct rchan *chan, > > unsigned int cpu, > > size_t consumed); > > diff --git a/kernel/relay.c b/kernel/relay.c > > index b5db4aa60da1..0e675a77285c 100644 > > --- a/kernel/relay.c > > +++ b/kernel/relay.c > > @@ -810,6 +810,41 @@ void relay_flush(struct rchan *chan) > > } > > EXPORT_SYMBOL_GPL(relay_flush); > > > > +/** > > + * relay_dump - dump statistics of the specified channel buffer > > + * @chan: the channel > > + * @buf: buf to store statistics > > + * @len: len of buf to check > > + * @flags: select particular information to dump > > + */ > > +void relay_dump(struct rchan *chan, char *buf, int len, int flags) > > +{ > > + unsigned int i, full_counter = 0; > > + struct rchan_buf *rbuf; > > + int offset = 0; > > + > > + if (!chan || !buf || flags & ~RELAY_DUMP_MASK) > > + return; > > + > > + if (len < RELAY_DUMP_BUF_MAX_LEN) > > + return; > > + > > + if (chan->is_global) { > > + rbuf = *per_cpu_ptr(chan->buf, 0); > > + full_counter = rbuf->stats.full; > > + } else { > > + for_each_possible_cpu(i) { > > + if ((rbuf = *per_cpu_ptr(chan->buf, i))) > > + full_counter += rbuf->stats.full; > > + } > > + > > + if (flags & RELAY_DUMP_BUF_FULL) > > + offset += snprintf(buf, sizeof(unsigned int), "%u", > > full_counter); > > + > > + snprintf(buf + offset, 1, "\n"); > > Is there any reason to return the value as string? > If it returns a digit value and the caller makes it a string, > it could be more flexible for other use cases.
Thanks for the feedback. I will remove the above one as you pointed out in the next revision. And it seems unnecessary to add '\0' at the end of the buffer like "*buf = '\0';"? While at it, I'm thinking if I can change the return value of relay_dump() to "how many bytes do we actually write into the buffer"? Does it sound better? Thanks, Jason > > Thank you, > > > +} > > +EXPORT_SYMBOL_GPL(relay_dump); > > + > > /** > > * relay_file_open - open file op for relay files > > * @inode: the inode > > -- > > 2.43.5 > > > > > -- > Masami Hiramatsu (Google) <[email protected]>
