Re: [patch V2 18/37] rseq: Provide static branch for runtime debugging

[Michael Jeanson <mjeanson@efficios.com>]

On 2025-08-23 12:39, Thomas Gleixner wrote:
> Config based debug is rarely turned on and is not available easily when
> things go wrong.
> 
> Provide a static branch to allow permanent integration of debug mechanisms
> along with the usual toggles in Kconfig, command line and debugfs.
> 
> Requested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>   Documentation/admin-guide/kernel-parameters.txt |    4 +
>   include/linux/rseq_entry.h                      |    3
>   init/Kconfig                                    |   14 ++++
>   kernel/rseq.c                                   |   73 ++++++++++++++++++++++--
>   4 files changed, 90 insertions(+), 4 deletions(-)
> 
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6443,6 +6443,10 @@
>   			Memory area to be used by remote processor image,
>   			managed by CMA.
>   
> +	rseq_debug=	[KNL] Enable or disable restartable sequence
> +			debug mode. Defaults to CONFIG_RSEQ_DEBUG_DEFAULT_ENABLE.
> +			Format: <bool>
> +
>   	rt_group_sched=	[KNL] Enable or disable SCHED_RR/FIFO group scheduling
>   			when CONFIG_RT_GROUP_SCHED=y. Defaults to
>   			!CONFIG_RT_GROUP_SCHED_DEFAULT_DISABLED.
> --- a/include/linux/rseq_entry.h
> +++ b/include/linux/rseq_entry.h
> @@ -34,6 +34,7 @@ DECLARE_PER_CPU(struct rseq_stats, rseq_
>   #endif /* !CONFIG_RSEQ_STATS */
>   
>   #ifdef CONFIG_RSEQ
> +#include <linux/jump_label.h>
>   #include <linux/rseq.h>
>   
>   #include <linux/tracepoint-defs.h>
> @@ -66,6 +67,8 @@ static inline void rseq_trace_ip_fixup(u
>   				       unsigned long offset, unsigned long abort_ip) { }
>   #endif /* !CONFIG_TRACEPOINT */
>   
> +DECLARE_STATIC_KEY_MAYBE(CONFIG_RSEQ_DEBUG_DEFAULT_ENABLE, rseq_debug_enabled);
> +
>   static __always_inline void rseq_note_user_irq_entry(void)
>   {
>   	if (IS_ENABLED(CONFIG_GENERIC_IRQ_ENTRY))
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1893,10 +1893,24 @@ config RSEQ_STATS
>   
>   	  If unsure, say N.
>   
> +config RSEQ_DEBUG_DEFAULT_ENABLE
> +	default n
> +	bool "Enable restartable sequences debug mode by default" if EXPERT
> +	depends on RSEQ
> +	help
> +	  This enables the static branch for debug mode of restartable
> +	  sequences.
> +
> +	  This also can be controlled on the kernel command line via the
> +	  command line parameter "rseq_debug=0/1" and through debugfs.
> +
> +	  If unsure, say N.
> +
>   config DEBUG_RSEQ
>   	default n
>   	bool "Enable debugging of rseq() system call" if EXPERT
>   	depends on RSEQ && DEBUG_KERNEL
> +	select RSEQ_DEBUG_DEFAULT_ENABLE
>   	help
>   	  Enable extra debugging checks for the rseq system call.
>   
> --- a/kernel/rseq.c
> +++ b/kernel/rseq.c
> @@ -95,6 +95,27 @@
>   				  RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL | \
>   				  RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE)
>   
> +DEFINE_STATIC_KEY_MAYBE(CONFIG_RSEQ_DEBUG_DEFAULT_ENABLE, rseq_debug_enabled);
> +
> +static inline void rseq_control_debug(bool on)
> +{
> +	if (on)
> +		static_branch_enable(&rseq_debug_enabled);
> +	else
> +		static_branch_disable(&rseq_debug_enabled);
> +}
> +
> +static int __init rseq_setup_debug(char *str)
> +{
> +	bool on;
> +
> +	if (kstrtobool(str, &on))
> +		return -EINVAL;
> +	rseq_control_debug(on);
> +	return 0;

Functions used by __setup() have to return '1' to signal that the 
argument was handled, otherwise you get this in the kernel log:

kernel: Unknown kernel command line parameters "rseq_debug=1", will be 
passed to user space.


> +}
> +__setup("rseq_debug=", rseq_setup_debug);
> +
>   #ifdef CONFIG_TRACEPOINTS
>   /*
>    * Out of line, so the actual update functions can be in a header to be
> @@ -112,10 +133,11 @@ void __rseq_trace_ip_fixup(unsigned long
>   }
>   #endif /* CONFIG_TRACEPOINTS */
>   
> +#ifdef CONFIG_DEBUG_FS
>   #ifdef CONFIG_RSEQ_STATS
>   DEFINE_PER_CPU(struct rseq_stats, rseq_stats);
>   
> -static int rseq_debug_show(struct seq_file *m, void *p)
> +static int rseq_stats_show(struct seq_file *m, void *p)
>   {
>   	struct rseq_stats stats = { };
>   	unsigned int cpu;
> @@ -140,14 +162,56 @@ static int rseq_debug_show(struct seq_fi
>   	return 0;
>   }
>   
> +static int rseq_stats_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, rseq_stats_show, inode->i_private);
> +}
> +
> +static const struct file_operations stat_ops = {
> +	.open		= rseq_stats_open,
> +	.read		= seq_read,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
> +static int __init rseq_stats_init(struct dentry *root_dir)
> +{
> +	debugfs_create_file("stats", 0444, root_dir, NULL, &stat_ops);
> +	return 0;
> +}
> +#else
> +static inline void rseq_stats_init(struct dentry *root_dir) { }
> +#endif /* CONFIG_RSEQ_STATS */
> +
> +static int rseq_debug_show(struct seq_file *m, void *p)
> +{
> +	bool on = static_branch_unlikely(&rseq_debug_enabled);
> +
> +	seq_printf(m, "%d\n", on);
> +	return 0;
> +}
> +
> +static ssize_t rseq_debug_write(struct file *file, const char __user *ubuf,
> +			    size_t count, loff_t *ppos)
> +{
> +	bool on;
> +
> +	if (kstrtobool_from_user(ubuf, count, &on))
> +		return -EINVAL;
> +
> +	rseq_control_debug(on);
> +	return count;
> +}
> +
>   static int rseq_debug_open(struct inode *inode, struct file *file)
>   {
>   	return single_open(file, rseq_debug_show, inode->i_private);
>   }
>   
> -static const struct file_operations dfs_ops = {
> +static const struct file_operations debug_ops = {
>   	.open		= rseq_debug_open,
>   	.read		= seq_read,
> +	.write		= rseq_debug_write,
>   	.llseek		= seq_lseek,
>   	.release	= single_release,
>   };
> @@ -156,11 +220,12 @@ static int __init rseq_debugfs_init(void
>   {
>   	struct dentry *root_dir = debugfs_create_dir("rseq", NULL);
>   
> -	debugfs_create_file("stats", 0444, root_dir, NULL, &dfs_ops);
> +	debugfs_create_file("debug", 0644, root_dir, NULL, &debug_ops);
> +	rseq_stats_init(root_dir);
>   	return 0;
>   }
>   __initcall(rseq_debugfs_init);
> -#endif /* CONFIG_RSEQ_STATS */
> +#endif /* CONFIG_DEBUG_FS */
>   
>   #ifdef CONFIG_DEBUG_RSEQ
>   static struct rseq *rseq_kernel_fields(struct task_struct *t)
> 
>