All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Mon, 22 Nov 2021 16:44:36 +0000	[thread overview]
Message-ID: <87v90kcf7v.mognet@arm.com> (raw)
In-Reply-To: <431fb6da-fe21-c5a6-bfb3-4e26bdc153b4@csgroup.eu>

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 :)

WARNING: multiple messages have this Message-ID (diff)
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>,
	Ingo Molnar <mingo@kernel.org>, Paul Mackerras <paulus@samba.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH v2 3/5] powerpc: Use preemption model accessors
Date: Mon, 22 Nov 2021 16:44:36 +0000	[thread overview]
Message-ID: <87v90kcf7v.mognet@arm.com> (raw)
In-Reply-To: <431fb6da-fe21-c5a6-bfb3-4e26bdc153b4@csgroup.eu>

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 :)

  reply	other threads:[~2021-11-22 16:44 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-10 20:24 [PATCH v2 0/5] preempt: PREEMPT vs PREEMPT_DYNAMIC configs fixup Valentin Schneider
2021-11-10 20:24 ` Valentin Schneider
2021-11-10 20:24 ` [PATCH v2 1/5] preempt: Restore preemption model selection configs Valentin Schneider
2021-11-10 20:24   ` Valentin Schneider
2021-11-11  8:58   ` Marco Elver
2021-11-11  8:58     ` Marco Elver
2021-11-11 12:22   ` [tip: sched/urgent] " tip-bot2 for Valentin Schneider
2021-11-10 20:24 ` [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors Valentin Schneider
2021-11-10 20:24   ` Valentin Schneider
2021-11-11  3:16   ` Mike Galbraith
2021-11-11  3:16     ` Mike Galbraith
2021-11-11  3:35     ` Mike Galbraith
2021-11-11  3:35       ` Mike Galbraith
2021-11-11  3:47       ` Mike Galbraith
2021-11-11  3:47         ` Mike Galbraith
2021-11-11  3:55         ` Mike Galbraith
2021-11-11  3:55           ` Mike Galbraith
2021-11-11  9:36         ` Marco Elver
2021-11-11  9:36           ` Marco Elver
2021-11-11 10:32           ` Mike Galbraith
2021-11-11 10:32             ` Mike Galbraith
2021-11-11 10:56             ` Valentin Schneider
2021-11-11 10:56               ` Valentin Schneider
2021-11-11 11:09               ` Mike Galbraith
2021-11-11 11:09                 ` Mike Galbraith
2021-11-11  8:54   ` Marco Elver
2021-11-11  8:54     ` Marco Elver
2021-11-11 10:56     ` Valentin Schneider
2021-11-11 10:56       ` Valentin Schneider
2021-11-16 13:29   ` Christophe Leroy
2021-11-16 13:29     ` Christophe Leroy
2021-11-22 16:37     ` Valentin Schneider
2021-11-22 16:37       ` Valentin Schneider
2021-11-10 20:24 ` [PATCH v2 3/5] powerpc: Use preemption model accessors Valentin Schneider
2021-11-10 20:24   ` Valentin Schneider
2021-11-11  4:55   ` Michael Ellerman
2021-11-11  4:55     ` Michael Ellerman
2021-11-15 15:29     ` Valentin Schneider
2021-11-15 15:29       ` Valentin Schneider
2021-11-16 13:41   ` Christophe Leroy
2021-11-16 13:41     ` Christophe Leroy
2021-11-22 16:44     ` Valentin Schneider [this message]
2021-11-22 16:44       ` Valentin Schneider
2021-11-10 20:24 ` [PATCH v2 4/5] kscan: " Valentin Schneider
2021-11-10 20:24   ` Valentin Schneider
2021-11-11  9:11   ` Marco Elver
2021-11-11  9:11     ` Marco Elver
2021-11-11  9:39     ` Marco Elver
2021-11-11  9:39       ` Marco Elver
2021-11-11 10:57     ` Valentin Schneider
2021-11-11 10:57       ` Valentin Schneider
2021-11-10 20:24 ` [PATCH v2 5/5] ftrace: Use preemption model accessors for trace header printout Valentin Schneider
2021-11-10 20:24   ` Valentin Schneider
2021-11-10 20:36   ` Steven Rostedt
2021-11-10 20:36     ` Steven Rostedt

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=87v90kcf7v.mognet@arm.com \
    --to=valentin.schneider@arm.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dvyukov@google.com \
    --cc=efault@gmx.de \
    --cc=elver@google.com \
    --cc=frederic@kernel.org \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=masahiroy@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=mingo@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.