[PATCH printk v4 8/8] printk: nbcon: Allow drivers to mark unsafe regions and check state

John Ogness posted 8 patches 2 years, 5 months ago
There is a newer version of this series
[PATCH printk v4 8/8] printk: nbcon: Allow drivers to mark unsafe regions and check state
Posted by John Ogness 2 years, 5 months ago
From: Thomas Gleixner <tglx@linutronix.de>

For the write_atomic callback, the console driver may have unsafe
regions that need to be appropriately marked. Provide functions
that accept the nbcon_write_context struct to allow for the driver
to enter and exit unsafe regions.

Also provide a function for drivers to check if they are still the
owner of the console.

Co-developed-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: Thomas Gleixner (Intel) <tglx@linutronix.de>
---
 include/linux/console.h | 10 +++++++
 kernel/printk/nbcon.c   | 66 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/include/linux/console.h b/include/linux/console.h
index 0ce7a2a856ab..de8fd92a960b 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -454,6 +454,16 @@ static inline bool console_is_registered(const struct console *con)
 	lockdep_assert_console_list_lock_held();			\
 	hlist_for_each_entry(con, &console_list, node)
 
+#ifdef CONFIG_PRINTK
+extern bool nbcon_can_proceed(struct nbcon_write_context *wctxt);
+extern bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt);
+extern bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt);
+#else
+static inline bool nbcon_can_proceed(struct nbcon_write_context *wctxt) { return false; }
+static inline bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt) { return false; }
+static inline bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt) { return false; }
+#endif
+
 extern int console_set_on_cmdline;
 extern struct console *early_console;
 
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index e2c274f4142e..04fac73c6e96 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -660,6 +660,32 @@ static bool nbcon_context_can_proceed(struct nbcon_context *ctxt, struct nbcon_s
 	return false;
 }
 
+/**
+ * nbcon_can_proceed - Check whether ownership can proceed
+ * @wctxt:	The write context that was handed to the write function
+ *
+ * Return:	True if this context still owns the console. False if
+ *		ownership was handed over or taken.
+ *
+ * Must be invoked at appropriate safe places in the driver.
+ *
+ * When this function returns false then the calling context no longer owns
+ * the console and is no longer allowed to go forward. In this case it must
+ * back out immediately and carefully. The buffer content is also no longer
+ * trusted since it no longer belongs to the calling context.
+ */
+bool nbcon_can_proceed(struct nbcon_write_context *wctxt)
+{
+	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
+	struct console *con = ctxt->console;
+	struct nbcon_state cur;
+
+	nbcon_state_read(con, &cur);
+
+	return nbcon_context_can_proceed(ctxt, &cur);
+}
+EXPORT_SYMBOL_GPL(nbcon_can_proceed);
+
 #define nbcon_context_enter_unsafe(c)	__nbcon_context_update_unsafe(c, true)
 #define nbcon_context_exit_unsafe(c)	__nbcon_context_update_unsafe(c, false)
 
@@ -710,6 +736,46 @@ static bool __nbcon_context_update_unsafe(struct nbcon_context *ctxt, bool unsaf
 	return nbcon_context_can_proceed(ctxt, &cur);
 }
 
+/**
+ * nbcon_enter_unsafe - Enter an unsafe region in the driver
+ * @wctxt:	The write context that was handed to the write function
+ *
+ * Return:	True if this context still owns the console. False if
+ *		ownership was handed over or taken.
+ *
+ * When this function returns false then the calling context no longer owns
+ * the console and is no longer allowed to go forward. In this case it must
+ * back out immediately and carefully. The buffer content is also no longer
+ * trusted since it no longer belongs to the calling context.
+ */
+bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt)
+{
+	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
+
+	return nbcon_context_enter_unsafe(ctxt);
+}
+EXPORT_SYMBOL_GPL(nbcon_enter_unsafe);
+
+/**
+ * nbcon_exit_unsafe - Exit an unsafe region in the driver
+ * @wctxt:	The write context that was handed to the write function
+ *
+ * Return:	True if this context still owns the console. False if
+ *		ownership was handed over or taken.
+ *
+ * When this function returns false then the calling context no longer owns
+ * the console and is no longer allowed to go forward. In this case it must
+ * back out immediately and carefully. The buffer content is also no longer
+ * trusted since it no longer belongs to the calling context.
+ */
+bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt)
+{
+	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
+
+	return nbcon_context_exit_unsafe(ctxt);
+}
+EXPORT_SYMBOL_GPL(nbcon_exit_unsafe);
+
 /**
  * nbcon_emit_next_record - Emit a record in the acquired context
  * @wctxt:	The write context that will be handed to the write function
-- 
2.39.2
Re: [PATCH printk v4 8/8] printk: nbcon: Allow drivers to mark unsafe regions and check state
Posted by Petr Mladek 2 years, 4 months ago
On Fri 2023-09-08 20:56:08, John Ogness wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> For the write_atomic callback, the console driver may have unsafe
> regions that need to be appropriately marked. Provide functions
> that accept the nbcon_write_context struct to allow for the driver
> to enter and exit unsafe regions.
> 
> Also provide a function for drivers to check if they are still the
> owner of the console.
> 
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index e2c274f4142e..04fac73c6e96 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -660,6 +660,32 @@ static bool nbcon_context_can_proceed(struct nbcon_context *ctxt, struct nbcon_s
>  	return false;
>  }
>  
> +/**
> + * nbcon_can_proceed - Check whether ownership can proceed
> + * @wctxt:	The write context that was handed to the write function
> + *
> + * Return:	True if this context still owns the console. False if
> + *		ownership was handed over or taken.
> + *
> + * Must be invoked at appropriate safe places in the driver.

This is a bit vague. I guess that enter_unsafe()/exit_unsafe() will be
used most of the time. The guestion is if this need to be called
in another locations explicitely.

I would write something similar as I suggested for nbcon_context_can_proceed():

  * It is used in nbcon_enter_unsafe() to make sure that it still owns the lock.
  * Also it is used in nbcon_exit_unsafe() to eventually free the lock
  * for a higher priority context which asked for the friendly handover.
  *
  * It can be called inside an unsafe section when the console is just
  * temporary in safe state instead of exiting and entering the unsafe
  * state.
  *
  * Also it can be called in the safe context before doing an expensive
  * safe operation. It does not make sense to do the operation when
  * a higher priority context took the lock.

> + *
> + * When this function returns false then the calling context no longer owns
> + * the console and is no longer allowed to go forward. In this case it must
> + * back out immediately and carefully. The buffer content is also no longer
> + * trusted since it no longer belongs to the calling context.
> + */
> +bool nbcon_can_proceed(struct nbcon_write_context *wctxt)
> +{
> +	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> +	struct console *con = ctxt->console;
> +	struct nbcon_state cur;
> +
> +	nbcon_state_read(con, &cur);
> +
> +	return nbcon_context_can_proceed(ctxt, &cur);
> +}
> +EXPORT_SYMBOL_GPL(nbcon_can_proceed);
> +

With the updated comment:

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

Best Regards,
Petr