On Fri, 21 Aug 2026 09:23:16 +0100 Bradley Morgan <[email protected]> wrote:
> On 21 August 2026 02:56:48 BST, Masami Hiramatsu <[email protected]> > wrote: > >On Thu, 20 Aug 2026 16:14:42 +0100 > >Bradley Morgan <[email protected]> wrote: > > > >> 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? > > > >Ah, OK. The phrasing aside, what you said wasn't wrong, so if you're happy > >with it, I'll pick it. (Let me pick it to probes/core) > > > > I'm happy with this patch as is, how accurate? > > Do the roar! (Pick it, Shrek reference) OK, I picked this to probes/core. (sorry for late notice) Thanks, > > >Thanks. > > > > > > > >> > >> > >> > > >> >>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! > > > > > > > > Thanks! -- Masami Hiramatsu (Google) <[email protected]>
