diff mbox series

[06/13] powerpc/rtas: clean up rtas_error_log_max initialization

Message ID 20221118150751.469393-7-nathanl@linux.ibm.com (mailing list archive)
State Accepted
Commit c67a0e411d0ffe0648fe84e25e9f899ce770feb3
Headers show
Series RTAS maintenance | expand

Commit Message

Nathan Lynch Nov. 18, 2022, 3:07 p.m. UTC 
The code in rtas_get_error_log_max() doesn't cause problems in
practice, but there are no measures to ensure that the lazy
initialization of the static rtas_error_log_max variable is atomic,
and it's not worth adding them.

Initialize the static rtas_error_log_max variable at boot when we're
single-threaded instead of lazily on first use. Use the more
appropriate of_property_read_u32() API instead of rtas_token() to
consult the "rtas-error-log-max" property, which is not the name of an
RTAS function. Convert use of printk() to pr_warn() and distinguish
the possible error cases.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
 arch/powerpc/kernel/rtas.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

Comments

Andrew Donnellan Nov. 22, 2022, 3:40 a.m. UTC | #1 
On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote:
> The code in rtas_get_error_log_max() doesn't cause problems in
> practice, but there are no measures to ensure that the lazy
> initialization of the static rtas_error_log_max variable is atomic,
> and it's not worth adding them.
> 
> Initialize the static rtas_error_log_max variable at boot when we're
> single-threaded instead of lazily on first use. Use the more
> appropriate of_property_read_u32() API instead of rtas_token() to
> consult the "rtas-error-log-max" property, which is not the name of
> an
> RTAS function. Convert use of printk() to pr_warn() and distinguish
> the possible error cases.
> 
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

> ---
>  arch/powerpc/kernel/rtas.c | 37 ++++++++++++++++++++++++++----------
> -
>  1 file changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 51f0508593a7..3fa84c247415 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -353,6 +353,9 @@ int rtas_service_present(const char *service)
>  EXPORT_SYMBOL(rtas_service_present);
>  
>  #ifdef CONFIG_RTAS_ERROR_LOGGING
> +
> +static u32 rtas_error_log_max __ro_after_init = RTAS_ERROR_LOG_MAX;
> +
>  /*
>   * Return the firmware-specified size of the error log buffer
>   *  for all rtas calls that require an error buffer argument.
> @@ -360,21 +363,30 @@ EXPORT_SYMBOL(rtas_service_present);
>   */
>  int rtas_get_error_log_max(void)
>  {
> -       static int rtas_error_log_max;
> -       if (rtas_error_log_max)
> -               return rtas_error_log_max;
> -
> -       rtas_error_log_max = rtas_token ("rtas-error-log-max");
> -       if ((rtas_error_log_max == RTAS_UNKNOWN_SERVICE) ||
> -           (rtas_error_log_max > RTAS_ERROR_LOG_MAX)) {
> -               printk (KERN_WARNING "RTAS: bad log buffer size
> %d\n",
> -                       rtas_error_log_max);
> -               rtas_error_log_max = RTAS_ERROR_LOG_MAX;
> -       }
>         return rtas_error_log_max;
>  }
>  EXPORT_SYMBOL(rtas_get_error_log_max);
>  
> +static void __init init_error_log_max(void)
> +{
> +       static const char propname[] __initconst = "rtas-error-log-
> max";
> +       u32 max;
> +
> +       if (of_property_read_u32(rtas.dev, propname, &max)) {
> +               pr_warn("%s not found, using default of %u\n",
> +                       propname, RTAS_ERROR_LOG_MAX);
> +               max = RTAS_ERROR_LOG_MAX;
> +       }
> +
> +       if (max > RTAS_ERROR_LOG_MAX) {
> +               pr_warn("%s = %u, clamping max error log size to
> %u\n",
> +                       propname, max, RTAS_ERROR_LOG_MAX);
> +               max = RTAS_ERROR_LOG_MAX;
> +       }
> +
> +       rtas_error_log_max = max;
> +}
> +
>  
>  static char rtas_err_buf[RTAS_ERROR_LOG_MAX];
>  static int rtas_last_error_token;
> @@ -432,6 +444,7 @@ static char *__fetch_rtas_last_error(char
> *altbuf)
>  #else /* CONFIG_RTAS_ERROR_LOGGING */
>  #define __fetch_rtas_last_error(x)     NULL
>  #define get_errorlog_buffer()          NULL
> +static void __init init_error_log_max(void) {}
>  #endif
>  
>  
> @@ -1341,6 +1354,8 @@ void __init rtas_initialize(void)
>         no_entry = of_property_read_u32(rtas.dev, "linux,rtas-entry",
> &entry);
>         rtas.entry = no_entry ? rtas.base : entry;
>  
> +       init_error_log_max();
> +
>         /*
>          * Discover these now to avoid device tree lookups in the
>          * panic path.
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 51f0508593a7..3fa84c247415 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -353,6 +353,9 @@  int rtas_service_present(const char *service)
 EXPORT_SYMBOL(rtas_service_present);
 
 #ifdef CONFIG_RTAS_ERROR_LOGGING
+
+static u32 rtas_error_log_max __ro_after_init = RTAS_ERROR_LOG_MAX;
+
 /*
  * Return the firmware-specified size of the error log buffer
  *  for all rtas calls that require an error buffer argument.
@@ -360,21 +363,30 @@  EXPORT_SYMBOL(rtas_service_present);
  */
 int rtas_get_error_log_max(void)
 {
-	static int rtas_error_log_max;
-	if (rtas_error_log_max)
-		return rtas_error_log_max;
-
-	rtas_error_log_max = rtas_token ("rtas-error-log-max");
-	if ((rtas_error_log_max == RTAS_UNKNOWN_SERVICE) ||
-	    (rtas_error_log_max > RTAS_ERROR_LOG_MAX)) {
-		printk (KERN_WARNING "RTAS: bad log buffer size %d\n",
-			rtas_error_log_max);
-		rtas_error_log_max = RTAS_ERROR_LOG_MAX;
-	}
 	return rtas_error_log_max;
 }
 EXPORT_SYMBOL(rtas_get_error_log_max);
 
+static void __init init_error_log_max(void)
+{
+	static const char propname[] __initconst = "rtas-error-log-max";
+	u32 max;
+
+	if (of_property_read_u32(rtas.dev, propname, &max)) {
+		pr_warn("%s not found, using default of %u\n",
+			propname, RTAS_ERROR_LOG_MAX);
+		max = RTAS_ERROR_LOG_MAX;
+	}
+
+	if (max > RTAS_ERROR_LOG_MAX) {
+		pr_warn("%s = %u, clamping max error log size to %u\n",
+			propname, max, RTAS_ERROR_LOG_MAX);
+		max = RTAS_ERROR_LOG_MAX;
+	}
+
+	rtas_error_log_max = max;
+}
+
 
 static char rtas_err_buf[RTAS_ERROR_LOG_MAX];
 static int rtas_last_error_token;
@@ -432,6 +444,7 @@  static char *__fetch_rtas_last_error(char *altbuf)
 #else /* CONFIG_RTAS_ERROR_LOGGING */
 #define __fetch_rtas_last_error(x)	NULL
 #define get_errorlog_buffer()		NULL
+static void __init init_error_log_max(void) {}
 #endif
 
 
@@ -1341,6 +1354,8 @@  void __init rtas_initialize(void)
 	no_entry = of_property_read_u32(rtas.dev, "linux,rtas-entry", &entry);
 	rtas.entry = no_entry ? rtas.base : entry;
 
+	init_error_log_max();
+
 	/*
 	 * Discover these now to avoid device tree lookups in the
 	 * panic path.