Re: [PATCH v2 1/2] printk: Add function to return log buffer address and size

[Vasant Hegde]

On 08/01/2014 03:52 AM, Andrew Morton wrote:
> On Thu, 31 Jul 2014 21:05:48 +0530 Vasant Hegde <hegdevasant@xxxxxxxxxxxxxxxxxx> wrote:
>
> > Platforms like IBM Power Systems supports service processor
> > assisted dump. It provides interface to add memory region to
> > be captured when system is crashed.
> >
> > During initialization/running we can add kernel memory region
> > to be collected.
> >
> > Presently we don't have a way to get the log buffer base address
> > and size. This patch adds support to return log buffer address
> > and size.
> >
> > ...
> >
> > --- a/include/linux/printk.h
> > +++ b/include/linux/printk.h
> > @@ -10,6 +10,9 @@
> >   extern const char linux_banner[];
> >   extern const char linux_proc_banner[];
> >
> > +extern void *get_log_buf_addr(void);
> > +extern u32 get_log_buf_len(void);
> > +
> >   static inline int printk_get_level(const char *buffer)
> >   {
> >   	if (buffer[0] == KERN_SOH_ASCII && buffer[1]) {
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 13e839d..4049f7b 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -270,6 +270,18 @@ static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
> >   static char *log_buf = __log_buf;
> >   static u32 log_buf_len = __LOG_BUF_LEN;
> >
> > +/* Return log buffer address */
> > +void *get_log_buf_addr(void)
> > +{
> > +	return log_buf;
> > +}
> > +
> > +/* Return log buffer size */
> > +u32 get_log_buf_len(void)
> > +{
> > +	return log_buf_len;
> > +}
> > +
> >   /* human readable text of the record */
> >   static char *log_text(const struct printk_log *msg)
> >   {

Andrew,

Thanks for the review.. Shall I add your Ack?

> Looks OK to me, although I think log_buf_addr_get() and
> log_buf_len_get() would be better names.  The kernel uses some
> big-endian names and some little-endian-names and some middle-endian
> names.  It's all rather a mess.  These symbols are already big-endian
> (ie: decreasing significance):
>
> 	log_buf -> addr -> get
> 	log_buf -> len -> get

Sure . Will rename function as above.

> The world wouldn't end if we simply made log_buf and log_buf_len global
> symbols.  The pros and cons are all minor.
>
> It's probably better to make get_log_buf_addr() tell the truth and
> return a char *.
>
>
> Please include this in whatever tree carries "powerpc/powernv:
> Interface to register/unregister opal dump region".

Sure.. Will work with BenH.

-Vasant

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/