From: Michael Ellerman <mpe@ellerman•id.au>
To: Christophe Leroy <christophe.leroy@csgroup•eu>,
"nathanl@linux•ibm.com" <nathanl@linux•ibm.com>,
Nicholas Piggin <npiggin@gmail•com>
Cc: "linuxppc-dev@lists•ozlabs.org" <linuxppc-dev@lists•ozlabs.org>
Subject: Re: [PATCH] powerpc/machdep: warn when machine_is() used too early
Date: Mon, 13 Feb 2023 21:45:04 +1100 [thread overview]
Message-ID: <87sff9vo7j.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <53a1e4be-e41a-8cb7-c617-b473a2a01b9d@csgroup.eu>
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
next prev parent reply other threads:[~2023-02-13 10:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20230210-warn-on-machine-is-before-probe-machine-v1-1-f0cba57125fb@linux.ibm.com>
2023-02-11 7:46 ` [PATCH] powerpc/machdep: warn when machine_is() used too early Christophe Leroy
2023-02-13 10:45 ` Michael Ellerman [this message]
2023-02-13 13:38 ` Nathan Lynch
2023-02-10 23:56 Nathan Lynch via B4 Submission Endpoint
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87sff9vo7j.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=nathanl@linux$(echo .)ibm.com \
--cc=npiggin@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox