From:   Valentin Schneider <valentin.schneider@arm.com>
To:     Christophe Leroy <christophe.leroy@csgroup.eu>,
        linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
        linuxppc-dev@lists.ozlabs.org, linux-kbuild@vger.kernel.org
Cc:     Marco Elver <elver@google.com>,
        Michal Marek <michal.lkml@markovi.net>,
        Peter Zijlstra <peterz@infradead.org>,
        Frederic Weisbecker <frederic@kernel.org>,
        Mike Galbraith <efault@gmx.de>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Paul Mackerras <paulus@samba.org>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH v2 3/5] powerpc: Use preemption model accessors
In-Reply-To: <431fb6da-fe21-c5a6-bfb3-4e26bdc153b4@csgroup.eu>
References: <20211110202448.4054153-1-valentin.schneider@arm.com> <20211110202448.4054153-4-valentin.schneider@arm.com> <431fb6da-fe21-c5a6-bfb3-4e26bdc153b4@csgroup.eu>
Date:   Mon, 22 Nov 2021 16:44:36 +0000
Message-ID: <87v90kcf7v.mognet@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On 16/11/21 14:41, Christophe Leroy wrote:
> Le 10/11/2021 à 21:24, Valentin Schneider a écrit :
>> Per PREEMPT_DYNAMIC, checking CONFIG_PREEMPT doesn't tell you the actual
>> preemption model of the live kernel. Use the newly-introduced accessors
>> instead.
>
> Is that change worth it for now ? As far as I can see powerpc doesn't
> have DYNAMIC PREEMPT, a lot of work needs to be done before being able
> to use it:
> - Implement GENERIC_ENTRY
> - Implement STATIC_CALLS (already done on PPC32, to be done on PPC64)
>

You're right, I ditched this patch for v3 - AFAICT the change wasn't even
valid as the preempt_schedule_irq() call needs to be replaced with
irqentry_exit_cond_resched() (IOW this needs to make use of the generic
entry code).

>>
>> sched_init() -> preempt_dynamic_init() happens way before IRQs are set up,
>> so this should be fine.
>
> It looks like you are mixing up interrupts and IRQs (also known as
> "external interrupts").
>
> ISI (Instruction Storage Interrupt) and DSI (Data Storage Interrupt) for
> instance are also interrupts. They happen everytime there is a page
> fault so may happen pretty early.
>
> Traps generated by WARN_ON() are also interrupts that may happen at any
> time.
>

Michael pointed this out and indeed triggering a WARN_ON() there is not
super smart. Thanks for teaching me a bit of what I'm putting my grubby
hands in :)