Re: [PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 18 Aug 2026 16:17:20 +0100
Bradley Morgan <include@grrlz.net> wrote:

> On 18 August 2026 03:01:18 BST, sashiko-bot@kernel.org wrote:
> >Thank you for your contribution! Sashiko AI review found 1 potential
> >issue(s) to consider:
> >- [Low] The 'enabled' debugfs file is created using `debugfs_create_file()` instead of `debugfs_create_file_unsafe()` with a `DEFINE_DEBUGFS_ATTRIBUTE` struct, leading to redundant proxying and overhead.  
> 
> +akpm@linux-foundation.org 
> 
> I hate sashiko so damn much, best to wait for review!

I found sashiko to be extremely useful. 


> >[ ... ]
> >  
> >> -static const struct file_operations fops_kp = {
> >> -	.read =         read_enabled_file_bool,
> >> -	.write =        write_enabled_file_bool,
> >> -	.llseek =	default_llseek,
> >> -};
> >> +DEFINE_DEBUGFS_ATTRIBUTE(fops_kp, kprobes_enabled_get,
> >> +			 kprobes_enabled_set, "%llu\n");
> >>  
> >>  static int __init debugfs_kprobe_init(void)
> >>  {  
> >
> >[Severity: Low]

It is even saying this is of "low priority". That means it's more of an "FYI".

> >Will this lead to double-proxying for the debugfs file?
> >
> >Since DEFINE_DEBUGFS_ATTRIBUTE natively provides debugfs_attr_read and
> >debugfs_attr_write operations that already include debugfs_file_get() and
> >debugfs_file_put() protection, passing fops_kp to debugfs_create_file()
> >in debugfs_kprobe_init() will cause the debugfs core to wrap it again:
> >
> >kernel/kprobes.c:debugfs_kprobe_init() {
> >    ...
> >	debugfs_create_file("enabled", 0600, dir, NULL, &fops_kp);
> >    ...
> >}
> >
> >Could this use debugfs_create_file_unsafe() instead?

As this isn't a critical path, I don't think we really care here if
it's wrapped or not. But it's nice to know that it is.

Anyway, I'll let others review this, but it looks fine to me.

-- Steve