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

2014-08-01 Thread Andrew Morton
On Thu, 31 Jul 2014 16:00:06 +0530 Vasant Hegde 
 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;
> +}
> +

This is the v1 patch.  The names are the same and get_log_buf_addr() still
returns void*.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-08-01 Thread Vasant Hegde

On 08/01/2014 03:52 AM, Andrew Morton wrote:

On Thu, 31 Jul 2014 21:05:48 +0530 Vasant Hegde 
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-08-01 Thread Vasant Hegde

On 08/01/2014 03:52 AM, Andrew Morton wrote:

On Thu, 31 Jul 2014 21:05:48 +0530 Vasant Hegde 
hegdevas...@linux.vnet.ibm.com 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-08-01 Thread Andrew Morton
On Thu, 31 Jul 2014 16:00:06 +0530 Vasant Hegde 
hegdevas...@linux.vnet.ibm.com 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;
 +}
 +

This is the v1 patch.  The names are the same and get_log_buf_addr() still
returns void*.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-07-31 Thread Joe Perches
On Thu, 2014-07-31 at 15:22 -0700, Andrew Morton wrote:

> Please include this in whatever tree carries "powerpc/powernv:
> Interface to register/unregister opal dump region".

At some point, I'd like to redo the patch series
that breaks up printk.c into more manageable blocks.

https://lkml.org/lkml/2012/10/17/41

Any suggestion for timing?

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


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

2014-07-31 Thread Andrew Morton
On Thu, 31 Jul 2014 21:05:48 +0530 Vasant Hegde 
 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)
>  {

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


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".

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


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

2014-07-31 Thread Andrew Morton
On Thu, 31 Jul 2014 21:05:48 +0530 Vasant Hegde 
hegdevas...@linux.vnet.ibm.com 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)
  {

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


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.

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


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

2014-07-31 Thread Joe Perches
On Thu, 2014-07-31 at 15:22 -0700, Andrew Morton wrote:

 Please include this in whatever tree carries powerpc/powernv:
 Interface to register/unregister opal dump region.

At some point, I'd like to redo the patch series
that breaks up printk.c into more manageable blocks.

https://lkml.org/lkml/2012/10/17/41

Any suggestion for timing?

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