| Message ID | 20230210-warn-on-machine-is-before-probe-machine-v2-1-b57f8243c51c@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 388defd5e4180a48e068d7ba9b024ce0ca957968 |
| Headers | show |
| Series | [v2] powerpc/machdep: warn when machine_is() used too early | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
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,
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)
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
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)