Thread (7 messages) 7 messages, 4 authors, 2026-08-18

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

flat view

From: Bradley Morgan <hidden>
Date: 2026-08-18 16:41:17
Also in: sashiko-reviews

On 18 August 2026 17:10:25 BST, Steven Rostedt [off-list ref] wrote:
On Tue, 18 Aug 2026 16:17:20 +0100
Bradley Morgan [off-list ref] wrote:
quoted
On 18 August 2026 03:01:18 BST, sashiko-bot@kernel.org wrote:
quoted
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.  
quoted
+akpm@linux-foundation.org 

I hate sashiko so damn much, best to wait for review!
I found sashiko to be extremely useful. 

quoted
quoted
[ ... ]
 
quoted
-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".
quoted
quoted
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
quoted
quoted
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
the fix is a couple of years in the making, heh
Thanks!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help