[PATCH printk 0/1] alternate fix for nbcon_cpu_emergency_flush()

[PATCH printk 0/1] alternate fix for nbcon_cpu_emergency_flush()

[John Ogness]

Hi Petr,

Currently in linux-next we have commit b955f0eead54 ("printk: nbcon:
Fix nbcon_cpu_emergency_flush() when preemptible"). But as you
pointed out [0], there is no need to disable preemption at all if
we only need to know if the current CPU is in emergency state.

Here is an alternative patch that correctly addresses the issue by
avoiding the migration checks.

Since the previous patch is not yet mainline, perhaps you want to
use this one instead in order to avoid a revert. Up to you.

John Ogness

[0] https://lore.kernel.org/lkml/Zn7b8g1HtuTIAwyi@pathway.suse.cz

John Ogness (1):
  printk: nbcon: do not require migration disabled for
    nbcon_get_cpu_emergency_nesting()

 kernel/printk/nbcon.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)


base-commit: c51cf7315db1392af58b909d1004133f6dbf0bf6
-- 
2.39.2

[PATCH printk 1/1] printk: nbcon: do not require migration disabled for nbcon_get_cpu_emergency_nesting()

[John Ogness]

Since entering emergency state disables preemption, there is no
need for nbcon_get_cpu_emergency_nesting() to explicitly require
migration disasbled. Either the current CPU is in emergency state
(and thus preemption is disabled and the nesting level is not
zero) or the current CPU is not in emergency state and the nesting
level must by definition be zero, even when migration is enabled
and the current CPU could change. The current CPU could never
change to a CPU that is in emergency state.

Reported-by: Juri Lelli <juri.lelli@redhat.com>
Closes: https://lore.kernel.org/aqkcpca4vgadxc3yzcu74xwq3grslj5m43f3eb5fcs23yo2gy4@gcsnqcts5tos
Fixes: 46a1379208b7 ("printk: nbcon: Implement emergency sections")
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/nbcon.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 6e9e24aa0a7f..ef6e76db0f5a 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -943,8 +943,14 @@ static unsigned int early_nbcon_pcpu_emergency_nesting __initdata;
 /**
  * nbcon_get_cpu_emergency_nesting - Get the per CPU emergency nesting pointer
  *
+ * Context:	For reading, any context. For writing, any context which could
+ *		not be migrated to another CPU.
  * Return:	Either a pointer to the per CPU emergency nesting counter of
  *		the current CPU or to the init data during early boot.
+ *
+ * Allowing migration enabled for reading relies on preemption being disabled
+ * while the current CPU is in the emergency state. See also
+ * nbcon_cpu_emergency_enter().
  */
 static __ref unsigned int *nbcon_get_cpu_emergency_nesting(void)
 {
@@ -956,7 +962,8 @@ static __ref unsigned int *nbcon_get_cpu_emergency_nesting(void)
 	if (!printk_percpu_data_ready())
 		return &early_nbcon_pcpu_emergency_nesting;
 
-	return this_cpu_ptr(&nbcon_pcpu_emergency_nesting);
+	/* Open code this_cpu_ptr() without checking migration. */
+	return per_cpu_ptr(&nbcon_pcpu_emergency_nesting, raw_smp_processor_id());
 }
 
 /**
-- 
2.39.2

Re: [PATCH printk 1/1] printk: nbcon: do not require migration disabled for nbcon_get_cpu_emergency_nesting()

[Petr Mladek]

On Fri 2024-07-19 15:54:18, John Ogness wrote:
> Since entering emergency state disables preemption, there is no
> need for nbcon_get_cpu_emergency_nesting() to explicitly require
> migration disasbled. Either the current CPU is in emergency state
> (and thus preemption is disabled and the nesting level is not
> zero) or the current CPU is not in emergency state and the nesting
> level must by definition be zero, even when migration is enabled
> and the current CPU could change. The current CPU could never
> change to a CPU that is in emergency state.
> 
> Reported-by: Juri Lelli <juri.lelli@redhat.com>
> Closes: https://lore.kernel.org/aqkcpca4vgadxc3yzcu74xwq3grslj5m43f3eb5fcs23yo2gy4@gcsnqcts5tos
> Fixes: 46a1379208b7 ("printk: nbcon: Implement emergency sections")
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH printk 0/1] alternate fix for nbcon_cpu_emergency_flush()

[Petr Mladek]

On Fri 2024-07-19 15:54:17, John Ogness wrote:
> Hi Petr,
> 
> Currently in linux-next we have commit b955f0eead54 ("printk: nbcon:
> Fix nbcon_cpu_emergency_flush() when preemptible"). But as you
> pointed out [0], there is no need to disable preemption at all if
> we only need to know if the current CPU is in emergency state.
> 
> Here is an alternative patch that correctly addresses the issue by
> avoiding the migration checks.
> 
> Since the previous patch is not yet mainline, perhaps you want to
> use this one instead in order to avoid a revert. Up to you.

Makes sense. I have replaced the commit b955f0eead54 ("printk: nbcon:
Fix nbcon_cpu_emergency_flush() when preemptible") with this new patch
in printk/linux.git, branch rework/write-atomic.

> [0] https://lore.kernel.org/lkml/Zn7b8g1HtuTIAwyi@pathway.suse.cz
> 
> John Ogness (1):
>   printk: nbcon: do not require migration disabled for
>     nbcon_get_cpu_emergency_nesting()
> 
>  kernel/printk/nbcon.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Best Regards,
Petr