All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] powerpc/machdep: warn when machine_is() used too early
@ 2023-02-13 19:23 Nathan Lynch
  2023-02-13 20:12 ` Christophe Leroy
  2023-02-20  3:49 ` Michael Ellerman
  0 siblings, 2 replies; 5+ messages in thread
From: Nathan Lynch @ 2023-02-13 19:23 UTC (permalink / raw)
  To: Michael Ellerman, Nicholas Piggin, Christophe Leroy
  Cc: linuxppc-dev, Nathan Lynch

machine_is() can't provide correct results before probe_machine() has
run. Warn when it's used too early in boot, placing the WARN_ON() in a
helper function so the reported file:line indicates exactly what went
wrong.

checkpatch complains about __attribute__((weak)) in the patch, so
change that to __weak, and align the line continuations as well.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
Prompted by my attempts to do some pseries-specific setup during
rtas_initialize() and being puzzled for a while that it wasn't
working.

Changes in v2:
- Use WARN_ON(), not WARN().
- Introduce __machine_is() helper function so the line reported is
  accurate.
- Update __attribute__((weak)) to __weak for checkpatch's sake.
- Link to v1: https://lore.kernel.org/r/20230210-warn-on-machine-is-before-probe-machine-v1-1-f0cba57125fb@linux.ibm.com
---
 arch/powerpc/include/asm/machdep.h | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 378b8d5836a7..459736d5e511 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -3,6 +3,7 @@
 #define _ASM_POWERPC_MACHDEP_H
 #ifdef __KERNEL__
 
+#include <linux/compiler.h>
 #include <linux/seq_file.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
@@ -220,11 +221,16 @@ extern struct machdep_calls *machine_id;
 	EXPORT_SYMBOL(mach_##name);				\
 	struct machdep_calls mach_##name __machine_desc =
 
-#define machine_is(name) \
-	({ \
-		extern struct machdep_calls mach_##name \
-			__attribute__((weak));		 \
-		machine_id == &mach_##name; \
+static inline bool __machine_is(const struct machdep_calls *md)
+{
+	WARN_ON(!machine_id); // complain if used before probe_machine()
+	return machine_id == md;
+}
+
+#define machine_is(name)                                        \
+	({                                                      \
+		extern struct machdep_calls mach_##name __weak; \
+		__machine_is(&mach_##name);                     \
 	})
 
 static inline void log_error(char *buf, unsigned int err_type, int fatal)

---
base-commit: 0bfb97203f5f300777624a2ad6f8f84aea3e8658
change-id: 20230210-warn-on-machine-is-before-probe-machine-37515b1f43bb

Best regards,
-- 
Nathan Lynch <nathanl@linux.ibm.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] powerpc/machdep: warn when machine_is() used too early
  2023-02-13 19:23 [PATCH v2] powerpc/machdep: warn when machine_is() used too early Nathan Lynch
@ 2023-02-13 20:12 ` Christophe Leroy
  2023-02-13 20:20   ` Nathan Lynch
  2023-02-20  3:49 ` Michael Ellerman
  1 sibling, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2023-02-13 20:12 UTC (permalink / raw)
  To: nathanl, Michael Ellerman, Nicholas Piggin; +Cc: linuxppc-dev



Le 13/02/2023 à 20:23, Nathan Lynch via B4 Submission Endpoint a écrit :
> From: Nathan Lynch <nathanl@linux.ibm.com>
> 
> machine_is() can't provide correct results before probe_machine() has
> run. Warn when it's used too early in boot, placing the WARN_ON() in a
> helper function so the reported file:line indicates exactly what went
> wrong.
> 
> checkpatch complains about __attribute__((weak)) in the patch, so
> change that to __weak, and align the line continuations as well.
> 
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
> Prompted by my attempts to do some pseries-specific setup during
> rtas_initialize() and being puzzled for a while that it wasn't
> working.
> 
> Changes in v2:
> - Use WARN_ON(), not WARN().
> - Introduce __machine_is() helper function so the line reported is
>    accurate.
> - Update __attribute__((weak)) to __weak for checkpatch's sake.
> - Link to v1: https://lore.kernel.org/r/20230210-warn-on-machine-is-before-probe-machine-v1-1-f0cba57125fb@linux.ibm.com
> ---
>   arch/powerpc/include/asm/machdep.h | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
> index 378b8d5836a7..459736d5e511 100644
> --- a/arch/powerpc/include/asm/machdep.h
> +++ b/arch/powerpc/include/asm/machdep.h
> @@ -3,6 +3,7 @@
>   #define _ASM_POWERPC_MACHDEP_H
>   #ifdef __KERNEL__
>   
> +#include <linux/compiler.h>
>   #include <linux/seq_file.h>
>   #include <linux/init.h>
>   #include <linux/dma-mapping.h>
> @@ -220,11 +221,16 @@ extern struct machdep_calls *machine_id;
>   	EXPORT_SYMBOL(mach_##name);				\
>   	struct machdep_calls mach_##name __machine_desc =
>   
> -#define machine_is(name) \
> -	({ \
> -		extern struct machdep_calls mach_##name \
> -			__attribute__((weak));		 \
> -		machine_id == &mach_##name; \
> +static inline bool __machine_is(const struct machdep_calls *md)
> +{
> +	WARN_ON(!machine_id); // complain if used before probe_machine()
> +	return machine_id == md;
> +}
> +
> +#define machine_is(name)                                        \

Misaligned back-slash ?

> +	({                                                      \
> +		extern struct machdep_calls mach_##name __weak; \
> +		__machine_is(&mach_##name);                     \
>   	})
>   
>   static inline void log_error(char *buf, unsigned int err_type, int fatal)
> 
> ---
> base-commit: 0bfb97203f5f300777624a2ad6f8f84aea3e8658
> change-id: 20230210-warn-on-machine-is-before-probe-machine-37515b1f43bb
> 
> Best regards,

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] powerpc/machdep: warn when machine_is() used too early
  2023-02-13 20:12 ` Christophe Leroy
@ 2023-02-13 20:20   ` Nathan Lynch
  0 siblings, 0 replies; 5+ messages in thread
From: Nathan Lynch @ 2023-02-13 20:20 UTC (permalink / raw)
  To: Christophe Leroy, Michael Ellerman, Nicholas Piggin; +Cc: linuxppc-dev

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 13/02/2023 à 20:23, Nathan Lynch via B4 Submission Endpoint a écrit :
>> From: Nathan Lynch <nathanl@linux.ibm.com>
>> 
>> machine_is() can't provide correct results before probe_machine() has
>> run. Warn when it's used too early in boot, placing the WARN_ON() in a
>> helper function so the reported file:line indicates exactly what went
>> wrong.
>> 
>> checkpatch complains about __attribute__((weak)) in the patch, so
>> change that to __weak, and align the line continuations as well.
>> 
>> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
>
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

thanks!

>> @@ -220,11 +221,16 @@ extern struct machdep_calls *machine_id;
>>   	EXPORT_SYMBOL(mach_##name);				\
>>   	struct machdep_calls mach_##name __machine_desc =
>>   
>> -#define machine_is(name) \
>> -	({ \
>> -		extern struct machdep_calls mach_##name \
>> -			__attribute__((weak));		 \
>> -		machine_id == &mach_##name; \
>> +static inline bool __machine_is(const struct machdep_calls *md)
>> +{
>> +	WARN_ON(!machine_id); // complain if used before probe_machine()
>> +	return machine_id == md;
>> +}
>> +
>> +#define machine_is(name)                                        \
>
> Misaligned back-slash ?

An artifact of the patch format + tabs for indentation. It should be
aligned with the rest when looking at the file itself after the patch is
applied.

>
>> +	({                                                      \
>> +		extern struct machdep_calls mach_##name __weak; \
>> +		__machine_is(&mach_##name);                     \
>>   	})
>>   
>>   static inline void log_error(char *buf, unsigned int err_type, int fatal)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] powerpc/machdep: warn when machine_is() used too early
  2023-02-13 19:23 [PATCH v2] powerpc/machdep: warn when machine_is() used too early Nathan Lynch
  2023-02-13 20:12 ` Christophe Leroy
@ 2023-02-20  3:49 ` Michael Ellerman
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2023-02-20  3:49 UTC (permalink / raw)
  To: Christophe Leroy, Nicholas Piggin, Michael Ellerman, Nathan Lynch
  Cc: linuxppc-dev

On Mon, 13 Feb 2023 13:23:51 -0600, Nathan Lynch wrote:
> machine_is() can't provide correct results before probe_machine() has
> run. Warn when it's used too early in boot, placing the WARN_ON() in a
> helper function so the reported file:line indicates exactly what went
> wrong.
> 
> checkpatch complains about __attribute__((weak)) in the patch, so
> change that to __weak, and align the line continuations as well.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/machdep: warn when machine_is() used too early
      https://git.kernel.org/powerpc/c/388defd5e4180a48e068d7ba9b024ce0ca957968

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] powerpc/machdep: warn when machine_is() used too early
@ 2023-02-13 19:23 Nathan Lynch via B4 Submission Endpoint
  0 siblings, 0 replies; 5+ messages in thread
From: Nathan Lynch via B4 Submission Endpoint @ 2023-02-13 19:23 UTC (permalink / raw)
  To: Michael Ellerman, Nicholas Piggin, Christophe Leroy
  Cc: Nathan Lynch, linuxppc-dev

From: Nathan Lynch <nathanl@linux.ibm.com>

machine_is() can't provide correct results before probe_machine() has
run. Warn when it's used too early in boot, placing the WARN_ON() in a
helper function so the reported file:line indicates exactly what went
wrong.

checkpatch complains about __attribute__((weak)) in the patch, so
change that to __weak, and align the line continuations as well.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
Prompted by my attempts to do some pseries-specific setup during
rtas_initialize() and being puzzled for a while that it wasn't
working.

Changes in v2:
- Use WARN_ON(), not WARN().
- Introduce __machine_is() helper function so the line reported is
  accurate.
- Update __attribute__((weak)) to __weak for checkpatch's sake.
- Link to v1: https://lore.kernel.org/r/20230210-warn-on-machine-is-before-probe-machine-v1-1-f0cba57125fb@linux.ibm.com
---
 arch/powerpc/include/asm/machdep.h | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 378b8d5836a7..459736d5e511 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -3,6 +3,7 @@
 #define _ASM_POWERPC_MACHDEP_H
 #ifdef __KERNEL__
 
+#include <linux/compiler.h>
 #include <linux/seq_file.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
@@ -220,11 +221,16 @@ extern struct machdep_calls *machine_id;
 	EXPORT_SYMBOL(mach_##name);				\
 	struct machdep_calls mach_##name __machine_desc =
 
-#define machine_is(name) \
-	({ \
-		extern struct machdep_calls mach_##name \
-			__attribute__((weak));		 \
-		machine_id == &mach_##name; \
+static inline bool __machine_is(const struct machdep_calls *md)
+{
+	WARN_ON(!machine_id); // complain if used before probe_machine()
+	return machine_id == md;
+}
+
+#define machine_is(name)                                        \
+	({                                                      \
+		extern struct machdep_calls mach_##name __weak; \
+		__machine_is(&mach_##name);                     \
 	})
 
 static inline void log_error(char *buf, unsigned int err_type, int fatal)

---
base-commit: 0bfb97203f5f300777624a2ad6f8f84aea3e8658
change-id: 20230210-warn-on-machine-is-before-probe-machine-37515b1f43bb

Best regards,
-- 
Nathan Lynch <nathanl@linux.ibm.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-02-20  3:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-13 19:23 [PATCH v2] powerpc/machdep: warn when machine_is() used too early Nathan Lynch
2023-02-13 20:12 ` Christophe Leroy
2023-02-13 20:20   ` Nathan Lynch
2023-02-20  3:49 ` Michael Ellerman
  -- strict thread matches above, loose matches on Subject: below --
2023-02-13 19:23 Nathan Lynch via B4 Submission Endpoint

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.