On 2019-10-16 06:57:48 [+0200], Christophe Leroy wrote:
>
>
> Le 15/10/2019 à 21:17, Sebastian Andrzej Siewior a écrit :
> > From: Thomas Gleixner <t...@linutronix.de>
> >
> > CONFIG_PREEMPTION is selected by CONFIG_PREEMPT and by CONFIG_PREEMPT_RT.
> > Both PREEMPT and PREEMPT_RT require the same functionality which today
> > depends on CONFIG_PREEMPT.
> >
> > Switch the entry code over to use CONFIG_PREEMPTION. Add PREEMPT_RT
> > output in __die().
>
> powerpc doesn't select ARCH_SUPPORTS_RT, so this change is useless as
> CONFIG_PREEMPT_RT cannot be selected.
No it is not. It makes it possible for PowerPC to select it one day and
I have patches for it today. Also, if other ARCH copies code from
PowerPC it will copy the correct thing (as in distinguish between the
flavour PREEMPT and the functionality PREEMPTION).
> > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> > index 82f43535e6867..23d2f20be4f2e 100644
> > --- a/arch/powerpc/kernel/traps.c
> > +++ b/arch/powerpc/kernel/traps.c
> > @@ -252,14 +252,19 @@ NOKPROBE_SYMBOL(oops_end);
> > static int __die(const char *str, struct pt_regs *regs, long err)
> > {
> > + const char *pr = "";
> > +
>
> Please follow the same approach as already existing. Don't add a local var
> for that.
I would leave it to the maintainer to comment on that and decide which
one they want. My eyes find it more readable and the compiles does not
create more code.
> > printk("Oops: %s, sig: %ld [#%d]\n", str, err, ++die_counter);
> > + if (IS_ENABLED(CONFIG_PREEMPTION))
> > + pr = IS_ENABLED(CONFIG_PREEMPT_RT) ? " PREEMPT_RT" : " PREEMPT";
> > +
>
> drop
>
> > printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s%s %s\n",
>
> Add one %s
>
> > IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
> > PAGE_SIZE / 1024,
> > early_radix_enabled() ? " MMU=Radix" : "",
> > early_mmu_has_feature(MMU_FTR_HPTE_TABLE) ? " MMU=Hash" : "",
> > - IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
>
> Replace by: IS_ENABLED(CONFIG_PREEMPTION) ? " PREEMPT" : ""
>
> > + pr,
>
> add something like: IS_ENABLED(CONFIG_PREEMPT_RT) ? "_RT" : ""
this on the other hand will create more code which is not strictly
required.
> > IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
> > IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
> > debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",
> >
>
> Christophe
Sebastian
