On 19 August 2026 00:12:40 BST, Bradley Morgan <[email protected]> wrote: >On 19 August 2026 00:08:13 BST, Masami Hiramatsu <[email protected]> >wrote: >>On Tue, 18 Aug 2026 01:53:18 +0000 >>Bradley Morgan <[email protected]> wrote: >> >>> This enabled knob is a disgusting terrible hack. It has rolled its >>> own read/write pair since 2007, writing '1' or '0' into a three byte >>> buffer by hand just to print a single character, with an XXX comment >>> begging debugfs for write callbacks on bool files. >>> DEFINE_DEBUGFS_ATTRIBUTE showed up in 2016 and does exactly that, so >>> the disgusting terrible hack has outlived its excuse for nine years. >>> Kill it, and the stale comment with it. >> >>In other words, does this mean something like the following? >> >>This "enable" knob implemented custom read/write logic, manually writing >>"1" or "0" to a 3-byte buffer solely to output a single character. As >>suggested by comments calling for `debugfs` to support write callbacks >>for boolean files, using `DEFINE_DEBUGFS_ATTRIBUTE` eliminates the need >>for these callbacks. >> > >ehhhhh, well, my description very well tells someone about why this is >bad, and a hack, and everything. > >Your description is good though... But I love to have my personality in my >descriptions :( >
Masami, could you give a round down of your nits for V3? > >>Thanks, >> >>> >>> The behavior does not change, except the write only accepts 0/1 now >>> instead of y/n/on/off, and nothing uses anything else. >>> >>> Signed-off-by: Bradley Morgan <[email protected]> >>> --- >>> Changes since v1: >>> - Added the missing Signed-off-by, sorry. >>> >>> kernel/kprobes.c | 43 +++++++++---------------------------------- >>> 1 file changed, 9 insertions(+), 34 deletions(-) >>> >>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c >>> index bfc89083daa9..044c6b5fd2aa 100644 >>> --- a/kernel/kprobes.c >>> +++ b/kernel/kprobes.c >>> @@ -3013,47 +3013,22 @@ static int disarm_all_kprobes(void) >>> return ret; >>> } >>> >>> -/* >>> - * XXX: The debugfs bool file interface doesn't allow for callbacks >>> - * when the bool state is switched. We can reuse that facility when >>> - * available >>> - */ >>> -static ssize_t read_enabled_file_bool(struct file *file, >>> - char __user *user_buf, size_t count, loff_t *ppos) >>> +static int kprobes_enabled_set(void *data, u64 val) >>> { >>> - char buf[3]; >>> + if (val) >>> + return arm_all_kprobes(); >>> >>> - if (!kprobes_all_disarmed) >>> - buf[0] = '1'; >>> - else >>> - buf[0] = '0'; >>> - buf[1] = '\n'; >>> - buf[2] = 0x00; >>> - return simple_read_from_buffer(user_buf, count, ppos, buf, 2); >>> + return disarm_all_kprobes(); >>> } >>> >>> -static ssize_t write_enabled_file_bool(struct file *file, >>> - const char __user *user_buf, size_t count, loff_t *ppos) >>> +static int kprobes_enabled_get(void *data, u64 *val) >>> { >>> - bool enable; >>> - int ret; >>> - >>> - ret = kstrtobool_from_user(user_buf, count, &enable); >>> - if (ret) >>> - return ret; >>> - >>> - ret = enable ? arm_all_kprobes() : disarm_all_kprobes(); >>> - if (ret) >>> - return ret; >>> - >>> - return count; >>> + *val = !kprobes_all_disarmed; >>> + return 0; >>> } >>> >>> -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) >>> { >>> -- >>> 2.47.3 >>> >> >> >> > >Thanks! Thanks!
