* [PATCH] powerpc/machdep: warn when machine_is() used too early
@ 2023-02-10 23:56 Nathan Lynch
2023-02-11 7:46 ` Christophe Leroy
0 siblings, 1 reply; 5+ messages in thread
From: Nathan Lynch @ 2023-02-10 23:56 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.
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.
---
arch/powerpc/include/asm/machdep.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 378b8d5836a7..8c0a799d18cd 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -220,11 +220,13 @@ 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; \
+#define machine_is(name) \
+ ({ \
+ extern struct machdep_calls mach_##name \
+ __attribute__((weak)); \
+ WARN(!machine_id, \
+ "machine_is() called before probe_machine()"); \
+ machine_id == &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] powerpc/machdep: warn when machine_is() used too early
2023-02-10 23:56 [PATCH] powerpc/machdep: warn when machine_is() used too early Nathan Lynch
@ 2023-02-11 7:46 ` Christophe Leroy
2023-02-13 10:45 ` Michael Ellerman
0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2023-02-11 7:46 UTC (permalink / raw)
To: nathanl@linux.ibm.com, Michael Ellerman, Nicholas Piggin
Cc: linuxppc-dev@lists.ozlabs.org
Le 11/02/2023 à 00:56, 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.
>
> 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.
> ---
> arch/powerpc/include/asm/machdep.h | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
> index 378b8d5836a7..8c0a799d18cd 100644
> --- a/arch/powerpc/include/asm/machdep.h
> +++ b/arch/powerpc/include/asm/machdep.h
> @@ -220,11 +220,13 @@ 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; \
> +#define machine_is(name) \
> + ({ \
> + extern struct machdep_calls mach_##name \
> + __attribute__((weak)); \
> + WARN(!machine_id, \
> + "machine_is() called before probe_machine()"); \
Is a WARN() really necessary ? WARN() is less optimised than WARN_ON(),
especially on PPC64.
This should never ever happen so a WARN_ON(!machine_id) should be
enough, the developper that hits it is able to go to the given file:line
and understand what happened.
> + machine_id == &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] powerpc/machdep: warn when machine_is() used too early
2023-02-11 7:46 ` Christophe Leroy
@ 2023-02-13 10:45 ` Michael Ellerman
2023-02-13 13:38 ` Nathan Lynch
0 siblings, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2023-02-13 10:45 UTC (permalink / raw)
To: Christophe Leroy, nathanl@linux.ibm.com, Nicholas Piggin
Cc: linuxppc-dev@lists.ozlabs.org
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 11/02/2023 à 00:56, 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.
>>
>> 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.
>> ---
>> arch/powerpc/include/asm/machdep.h | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>> index 378b8d5836a7..8c0a799d18cd 100644
>> --- a/arch/powerpc/include/asm/machdep.h
>> +++ b/arch/powerpc/include/asm/machdep.h
>> @@ -220,11 +220,13 @@ 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; \
>> +#define machine_is(name) \
>> + ({ \
>> + extern struct machdep_calls mach_##name \
>> + __attribute__((weak)); \
>> + WARN(!machine_id, \
>> + "machine_is() called before probe_machine()"); \
>
> Is a WARN() really necessary ? WARN() is less optimised than WARN_ON(),
> especially on PPC64.
>
> This should never ever happen so a WARN_ON(!machine_id) should be
> enough, the developper that hits it is able to go to the given file:line
> and understand what happened.
Yeah I agree, WARN_ON() should be sufficient here, and should generate
slightly better code. We have > 100 uses of machine_is(), so keeping
each small is desirable.
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/machdep: warn when machine_is() used too early
2023-02-13 10:45 ` Michael Ellerman
@ 2023-02-13 13:38 ` Nathan Lynch
0 siblings, 0 replies; 5+ messages in thread
From: Nathan Lynch @ 2023-02-13 13:38 UTC (permalink / raw)
To: Michael Ellerman, Christophe Leroy, Nicholas Piggin
Cc: linuxppc-dev@lists.ozlabs.org
Michael Ellerman <mpe@ellerman.id.au> writes:
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 11/02/2023 à 00:56, 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.
>>>
>>> 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.
>>> ---
>>> arch/powerpc/include/asm/machdep.h | 12 +++++++-----
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>>> index 378b8d5836a7..8c0a799d18cd 100644
>>> --- a/arch/powerpc/include/asm/machdep.h
>>> +++ b/arch/powerpc/include/asm/machdep.h
>>> @@ -220,11 +220,13 @@ 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; \
>>> +#define machine_is(name) \
>>> + ({ \
>>> + extern struct machdep_calls mach_##name \
>>> + __attribute__((weak)); \
>>> + WARN(!machine_id, \
>>> + "machine_is() called before probe_machine()"); \
>>
>> Is a WARN() really necessary ? WARN() is less optimised than WARN_ON(),
>> especially on PPC64.
>>
>> This should never ever happen so a WARN_ON(!machine_id) should be
>> enough, the developper that hits it is able to go to the given file:line
>> and understand what happened.
>
> Yeah I agree, WARN_ON() should be sufficient here, and should generate
> slightly better code. We have > 100 uses of machine_is(), so keeping
> each small is desirable.
Sure, I'll change it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] powerpc/machdep: warn when machine_is() used too early
@ 2023-02-10 23:56 Nathan Lynch via B4 Submission Endpoint
0 siblings, 0 replies; 5+ messages in thread
From: Nathan Lynch via B4 Submission Endpoint @ 2023-02-10 23:56 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.
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.
---
arch/powerpc/include/asm/machdep.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 378b8d5836a7..8c0a799d18cd 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -220,11 +220,13 @@ 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; \
+#define machine_is(name) \
+ ({ \
+ extern struct machdep_calls mach_##name \
+ __attribute__((weak)); \
+ WARN(!machine_id, \
+ "machine_is() called before probe_machine()"); \
+ machine_id == &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-13 13:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-10 23:56 [PATCH] powerpc/machdep: warn when machine_is() used too early Nathan Lynch
2023-02-11 7:46 ` Christophe Leroy
2023-02-13 10:45 ` Michael Ellerman
2023-02-13 13:38 ` Nathan Lynch
-- strict thread matches above, loose matches on Subject: below --
2023-02-10 23:56 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.