| Message ID | 20240625002144.3485799-6-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | uprobes: add batched register/unregister APIs and per-CPU RW semaphore | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Mon, 24 Jun 2024 17:21:37 -0700 Andrii Nakryiko <andrii@kernel.org> wrote: > Simplify uprobe registration/unregistration interfaces by making offset > and ref_ctr_offset part of uprobe_consumer "interface". In practice, all > existing users already store these fields somewhere in uprobe_consumer's > containing structure, so this doesn't pose any problem. We just move > some fields around. > > On the other hand, this simplifies uprobe_register() and > uprobe_unregister() API by having only struct uprobe_consumer as one > thing representing attachment/detachment entity. This makes batched > versions of uprobe_register() and uprobe_unregister() simpler. > > This also makes uprobe_register_refctr() unnecessary, so remove it and > simplify consumers. > > No functional changes intended. > Looks good to me. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thanks! > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > include/linux/uprobes.h | 18 +++---- > kernel/events/uprobes.c | 19 ++----- > kernel/trace/bpf_trace.c | 21 +++----- > kernel/trace/trace_uprobe.c | 53 ++++++++----------- > .../selftests/bpf/bpf_testmod/bpf_testmod.c | 23 ++++---- > 5 files changed, 55 insertions(+), 79 deletions(-) > > diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h > index b503fafb7fb3..a75ba37ce3c8 100644 > --- a/include/linux/uprobes.h > +++ b/include/linux/uprobes.h > @@ -42,6 +42,11 @@ struct uprobe_consumer { > enum uprobe_filter_ctx ctx, > struct mm_struct *mm); > > + /* associated file offset of this probe */ > + loff_t offset; > + /* associated refctr file offset of this probe, or zero */ > + loff_t ref_ctr_offset; > + /* for internal uprobe infra use, consumers shouldn't touch fields below */ > struct uprobe_consumer *next; > }; > > @@ -110,10 +115,9 @@ extern bool is_trap_insn(uprobe_opcode_t *insn); > extern unsigned long uprobe_get_swbp_addr(struct pt_regs *regs); > extern unsigned long uprobe_get_trap_addr(struct pt_regs *regs); > extern int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_t); > -extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc); > -extern int uprobe_register_refctr(struct inode *inode, loff_t offset, loff_t ref_ctr_offset, struct uprobe_consumer *uc); > +extern int uprobe_register(struct inode *inode, struct uprobe_consumer *uc); > extern int uprobe_apply(struct inode *inode, loff_t offset, struct uprobe_consumer *uc, bool); > -extern void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *uc); > +extern void uprobe_unregister(struct inode *inode, struct uprobe_consumer *uc); > extern int uprobe_mmap(struct vm_area_struct *vma); > extern void uprobe_munmap(struct vm_area_struct *vma, unsigned long start, unsigned long end); > extern void uprobe_start_dup_mmap(void); > @@ -152,11 +156,7 @@ static inline void uprobes_init(void) > #define uprobe_get_trap_addr(regs) instruction_pointer(regs) > > static inline int > -uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc) > -{ > - return -ENOSYS; > -} > -static inline int uprobe_register_refctr(struct inode *inode, loff_t offset, loff_t ref_ctr_offset, struct uprobe_consumer *uc) > +uprobe_register(struct inode *inode, struct uprobe_consumer *uc) > { > return -ENOSYS; > } > @@ -166,7 +166,7 @@ uprobe_apply(struct inode *inode, loff_t offset, struct uprobe_consumer *uc, boo > return -ENOSYS; > } > static inline void > -uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *uc) > +uprobe_unregister(struct inode *inode, struct uprobe_consumer *uc) > { > } > static inline int uprobe_mmap(struct vm_area_struct *vma) > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 8ce669bc6474..2544e8b79bad 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -1197,14 +1197,13 @@ __uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *uc) > /* > * uprobe_unregister - unregister an already registered probe. > * @inode: the file in which the probe has to be removed. > - * @offset: offset from the start of the file. > - * @uc: identify which probe if multiple probes are colocated. > + * @uc: identify which probe consumer to unregister. > */ > -void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *uc) > +void uprobe_unregister(struct inode *inode, struct uprobe_consumer *uc) > { > struct uprobe *uprobe; > > - uprobe = find_uprobe(inode, offset); > + uprobe = find_uprobe(inode, uc->offset); > if (WARN_ON(!uprobe)) > return; > > @@ -1277,20 +1276,12 @@ static int __uprobe_register(struct inode *inode, loff_t offset, > return ret; > } > > -int uprobe_register(struct inode *inode, loff_t offset, > - struct uprobe_consumer *uc) > +int uprobe_register(struct inode *inode, struct uprobe_consumer *uc) > { > - return __uprobe_register(inode, offset, 0, uc); > + return __uprobe_register(inode, uc->offset, uc->ref_ctr_offset, uc); > } > EXPORT_SYMBOL_GPL(uprobe_register); > > -int uprobe_register_refctr(struct inode *inode, loff_t offset, > - loff_t ref_ctr_offset, struct uprobe_consumer *uc) > -{ > - return __uprobe_register(inode, offset, ref_ctr_offset, uc); > -} > -EXPORT_SYMBOL_GPL(uprobe_register_refctr); > - > /* > * uprobe_apply - unregister an already registered probe. > * @inode: the file in which the probe has to be removed. > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index d1daeab1bbc1..ba62baec3152 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -3154,8 +3154,6 @@ struct bpf_uprobe_multi_link; > > struct bpf_uprobe { > struct bpf_uprobe_multi_link *link; > - loff_t offset; > - unsigned long ref_ctr_offset; > u64 cookie; > struct uprobe_consumer consumer; > }; > @@ -3181,8 +3179,7 @@ static void bpf_uprobe_unregister(struct path *path, struct bpf_uprobe *uprobes, > u32 i; > > for (i = 0; i < cnt; i++) { > - uprobe_unregister(d_real_inode(path->dentry), uprobes[i].offset, > - &uprobes[i].consumer); > + uprobe_unregister(d_real_inode(path->dentry), &uprobes[i].consumer); > } > } > > @@ -3262,10 +3259,10 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, > > for (i = 0; i < ucount; i++) { > if (uoffsets && > - put_user(umulti_link->uprobes[i].offset, uoffsets + i)) > + put_user(umulti_link->uprobes[i].consumer.offset, uoffsets + i)) > return -EFAULT; > if (uref_ctr_offsets && > - put_user(umulti_link->uprobes[i].ref_ctr_offset, uref_ctr_offsets + i)) > + put_user(umulti_link->uprobes[i].consumer.ref_ctr_offset, uref_ctr_offsets + i)) > return -EFAULT; > if (ucookies && > put_user(umulti_link->uprobes[i].cookie, ucookies + i)) > @@ -3439,15 +3436,16 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > goto error_free; > > for (i = 0; i < cnt; i++) { > - if (__get_user(uprobes[i].offset, uoffsets + i)) { > + if (__get_user(uprobes[i].consumer.offset, uoffsets + i)) { > err = -EFAULT; > goto error_free; > } > - if (uprobes[i].offset < 0) { > + if (uprobes[i].consumer.offset < 0) { > err = -EINVAL; > goto error_free; > } > - if (uref_ctr_offsets && __get_user(uprobes[i].ref_ctr_offset, uref_ctr_offsets + i)) { > + if (uref_ctr_offsets && > + __get_user(uprobes[i].consumer.ref_ctr_offset, uref_ctr_offsets + i)) { > err = -EFAULT; > goto error_free; > } > @@ -3477,10 +3475,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > &bpf_uprobe_multi_link_lops, prog); > > for (i = 0; i < cnt; i++) { > - err = uprobe_register_refctr(d_real_inode(link->path.dentry), > - uprobes[i].offset, > - uprobes[i].ref_ctr_offset, > - &uprobes[i].consumer); > + err = uprobe_register(d_real_inode(link->path.dentry), &uprobes[i].consumer); > if (err) { > bpf_uprobe_unregister(&path, uprobes, i); > goto error_free; > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index c98e3b3386ba..d786f99114be 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -60,8 +60,6 @@ struct trace_uprobe { > struct path path; > struct inode *inode; > char *filename; > - unsigned long offset; > - unsigned long ref_ctr_offset; > unsigned long nhit; > struct trace_probe tp; > }; > @@ -205,7 +203,7 @@ static unsigned long translate_user_vaddr(unsigned long file_offset) > > udd = (void *) current->utask->vaddr; > > - base_addr = udd->bp_addr - udd->tu->offset; > + base_addr = udd->bp_addr - udd->tu->consumer.offset; > return base_addr + file_offset; > } > > @@ -286,13 +284,13 @@ static bool trace_uprobe_match_command_head(struct trace_uprobe *tu, > if (strncmp(tu->filename, argv[0], len) || argv[0][len] != ':') > return false; > > - if (tu->ref_ctr_offset == 0) > - snprintf(buf, sizeof(buf), "0x%0*lx", > - (int)(sizeof(void *) * 2), tu->offset); > + if (tu->consumer.ref_ctr_offset == 0) > + snprintf(buf, sizeof(buf), "0x%0*llx", > + (int)(sizeof(void *) * 2), tu->consumer.offset); > else > - snprintf(buf, sizeof(buf), "0x%0*lx(0x%lx)", > - (int)(sizeof(void *) * 2), tu->offset, > - tu->ref_ctr_offset); > + snprintf(buf, sizeof(buf), "0x%0*llx(0x%llx)", > + (int)(sizeof(void *) * 2), tu->consumer.offset, > + tu->consumer.ref_ctr_offset); > if (strcmp(buf, &argv[0][len + 1])) > return false; > > @@ -410,7 +408,7 @@ static bool trace_uprobe_has_same_uprobe(struct trace_uprobe *orig, > > list_for_each_entry(orig, &tpe->probes, tp.list) { > if (comp_inode != d_real_inode(orig->path.dentry) || > - comp->offset != orig->offset) > + comp->consumer.offset != orig->consumer.offset) > continue; > > /* > @@ -472,8 +470,8 @@ static int validate_ref_ctr_offset(struct trace_uprobe *new) > > for_each_trace_uprobe(tmp, pos) { > if (new_inode == d_real_inode(tmp->path.dentry) && > - new->offset == tmp->offset && > - new->ref_ctr_offset != tmp->ref_ctr_offset) { > + new->consumer.offset == tmp->consumer.offset && > + new->consumer.ref_ctr_offset != tmp->consumer.ref_ctr_offset) { > pr_warn("Reference counter offset mismatch."); > return -EINVAL; > } > @@ -675,8 +673,8 @@ static int __trace_uprobe_create(int argc, const char **argv) > WARN_ON_ONCE(ret != -ENOMEM); > goto fail_address_parse; > } > - tu->offset = offset; > - tu->ref_ctr_offset = ref_ctr_offset; > + tu->consumer.offset = offset; > + tu->consumer.ref_ctr_offset = ref_ctr_offset; > tu->path = path; > tu->filename = filename; > > @@ -746,12 +744,12 @@ static int trace_uprobe_show(struct seq_file *m, struct dyn_event *ev) > char c = is_ret_probe(tu) ? 'r' : 'p'; > int i; > > - seq_printf(m, "%c:%s/%s %s:0x%0*lx", c, trace_probe_group_name(&tu->tp), > + seq_printf(m, "%c:%s/%s %s:0x%0*llx", c, trace_probe_group_name(&tu->tp), > trace_probe_name(&tu->tp), tu->filename, > - (int)(sizeof(void *) * 2), tu->offset); > + (int)(sizeof(void *) * 2), tu->consumer.offset); > > - if (tu->ref_ctr_offset) > - seq_printf(m, "(0x%lx)", tu->ref_ctr_offset); > + if (tu->consumer.ref_ctr_offset) > + seq_printf(m, "(0x%llx)", tu->consumer.ref_ctr_offset); > > for (i = 0; i < tu->tp.nr_args; i++) > seq_printf(m, " %s=%s", tu->tp.args[i].name, tu->tp.args[i].comm); > @@ -1089,12 +1087,7 @@ static int trace_uprobe_enable(struct trace_uprobe *tu, filter_func_t filter) > tu->consumer.filter = filter; > tu->inode = d_real_inode(tu->path.dentry); > > - if (tu->ref_ctr_offset) > - ret = uprobe_register_refctr(tu->inode, tu->offset, > - tu->ref_ctr_offset, &tu->consumer); > - else > - ret = uprobe_register(tu->inode, tu->offset, &tu->consumer); > - > + ret = uprobe_register(tu->inode, &tu->consumer); > if (ret) > tu->inode = NULL; > > @@ -1112,7 +1105,7 @@ static void __probe_event_disable(struct trace_probe *tp) > if (!tu->inode) > continue; > > - uprobe_unregister(tu->inode, tu->offset, &tu->consumer); > + uprobe_unregister(tu->inode, &tu->consumer); > tu->inode = NULL; > } > } > @@ -1310,7 +1303,7 @@ static int uprobe_perf_close(struct trace_event_call *call, > return 0; > > list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) { > - ret = uprobe_apply(tu->inode, tu->offset, &tu->consumer, false); > + ret = uprobe_apply(tu->inode, tu->consumer.offset, &tu->consumer, false); > if (ret) > break; > } > @@ -1334,7 +1327,7 @@ static int uprobe_perf_open(struct trace_event_call *call, > return 0; > > list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) { > - err = uprobe_apply(tu->inode, tu->offset, &tu->consumer, true); > + err = uprobe_apply(tu->inode, tu->consumer.offset, &tu->consumer, true); > if (err) { > uprobe_perf_close(call, event); > break; > @@ -1464,7 +1457,7 @@ int bpf_get_uprobe_info(const struct perf_event *event, u32 *fd_type, > *fd_type = is_ret_probe(tu) ? BPF_FD_TYPE_URETPROBE > : BPF_FD_TYPE_UPROBE; > *filename = tu->filename; > - *probe_offset = tu->offset; > + *probe_offset = tu->consumer.offset; > *probe_addr = 0; > return 0; > } > @@ -1627,9 +1620,9 @@ create_local_trace_uprobe(char *name, unsigned long offs, > return ERR_CAST(tu); > } > > - tu->offset = offs; > + tu->consumer.offset = offs; > tu->path = path; > - tu->ref_ctr_offset = ref_ctr_offset; > + tu->consumer.ref_ctr_offset = ref_ctr_offset; > tu->filename = kstrdup(name, GFP_KERNEL); > if (!tu->filename) { > ret = -ENOMEM; > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > index b0132a342bb5..9ae2a3c64daa 100644 > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > @@ -377,7 +377,6 @@ uprobe_ret_handler(struct uprobe_consumer *self, unsigned long func, > > struct testmod_uprobe { > struct path path; > - loff_t offset; > struct uprobe_consumer consumer; > }; > > @@ -391,25 +390,24 @@ static int testmod_register_uprobe(loff_t offset) > { > int err = -EBUSY; > > - if (uprobe.offset) > + if (uprobe.consumer.offset) > return -EBUSY; > > mutex_lock(&testmod_uprobe_mutex); > > - if (uprobe.offset) > + if (uprobe.consumer.offset) > goto out; > > err = kern_path("/proc/self/exe", LOOKUP_FOLLOW, &uprobe.path); > if (err) > goto out; > > - err = uprobe_register_refctr(d_real_inode(uprobe.path.dentry), > - offset, 0, &uprobe.consumer); > - if (err) > + uprobe.consumer.offset = offset; > + err = uprobe_register(d_real_inode(uprobe.path.dentry), &uprobe.consumer); > + if (err) { > path_put(&uprobe.path); > - else > - uprobe.offset = offset; > - > + uprobe.consumer.offset = 0; > + } > out: > mutex_unlock(&testmod_uprobe_mutex); > return err; > @@ -419,10 +417,9 @@ static void testmod_unregister_uprobe(void) > { > mutex_lock(&testmod_uprobe_mutex); > > - if (uprobe.offset) { > - uprobe_unregister(d_real_inode(uprobe.path.dentry), > - uprobe.offset, &uprobe.consumer); > - uprobe.offset = 0; > + if (uprobe.consumer.offset) { > + uprobe_unregister(d_real_inode(uprobe.path.dentry), &uprobe.consumer); > + uprobe.consumer.offset = 0; > } > > mutex_unlock(&testmod_uprobe_mutex); > -- > 2.43.0 >
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h index b503fafb7fb3..a75ba37ce3c8 100644 --- a/include/linux/uprobes.h +++ b/include/linux/uprobes.h @@ -42,6 +42,11 @@ struct uprobe_consumer { enum uprobe_filter_ctx ctx, struct mm_struct *mm); + /* associated file offset of this probe */ + loff_t offset; + /* associated refctr file offset of this probe, or zero */ + loff_t ref_ctr_offset; + /* for internal uprobe infra use, consumers shouldn't touch fields below */ struct uprobe_consumer *next; }; @@ -110,10 +115,9 @@ extern bool is_trap_insn(uprobe_opcode_t *insn); extern unsigned long uprobe_get_swbp_addr(struct pt_regs *regs); extern unsigned long uprobe_get_trap_addr(struct pt_regs *regs); extern int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_t); -extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc); -extern int uprobe_register_refctr(struct inode *inode, loff_t offset, loff_t ref_ctr_offset, struct uprobe_consumer *uc); +extern int uprobe_register(struct inode *inode, struct uprobe_consumer *uc); extern int uprobe_apply(struct inode *inode, loff_t offset, struct uprobe_consumer *uc, bool); -extern void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *uc); +extern void uprobe_unregister(struct inode *inode, struct uprobe_consumer *uc); extern int uprobe_mmap(struct vm_area_struct *vma); extern void uprobe_munmap(struct vm_area_struct *vma, unsigned long start, unsigned long end); extern void uprobe_start_dup_mmap(void); @@ -152,11 +156,7 @@ static inline void uprobes_init(void) #define uprobe_get_trap_addr(regs) instruction_pointer(regs) static inline int -uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc) -{ - return -ENOSYS; -} -static inline int uprobe_register_refctr(struct inode *inode, loff_t offset, loff_t ref_ctr_offset, struct uprobe_consumer *uc) +uprobe_register(struct inode *inode, struct uprobe_consumer *uc) { return -ENOSYS; } @@ -166,7 +166,7 @@ uprobe_apply(struct inode *inode, loff_t offset, struct uprobe_consumer *uc, boo return -ENOSYS; } static inline void -uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *uc) +uprobe_unregister(struct inode *inode, struct uprobe_consumer *uc) { } static inline int uprobe_mmap(struct vm_area_struct *vma) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 8ce669bc6474..2544e8b79bad 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -1197,14 +1197,13 @@ __uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *uc) /* * uprobe_unregister - unregister an already registered probe. * @inode: the file in which the probe has to be removed. - * @offset: offset from the start of the file. - * @uc: identify which probe if multiple probes are colocated. + * @uc: identify which probe consumer to unregister. */ -void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *uc) +void uprobe_unregister(struct inode *inode, struct uprobe_consumer *uc) { struct uprobe *uprobe; - uprobe = find_uprobe(inode, offset); + uprobe = find_uprobe(inode, uc->offset); if (WARN_ON(!uprobe)) return; @@ -1277,20 +1276,12 @@ static int __uprobe_register(struct inode *inode, loff_t offset, return ret; } -int uprobe_register(struct inode *inode, loff_t offset, - struct uprobe_consumer *uc) +int uprobe_register(struct inode *inode, struct uprobe_consumer *uc) { - return __uprobe_register(inode, offset, 0, uc); + return __uprobe_register(inode, uc->offset, uc->ref_ctr_offset, uc); } EXPORT_SYMBOL_GPL(uprobe_register); -int uprobe_register_refctr(struct inode *inode, loff_t offset, - loff_t ref_ctr_offset, struct uprobe_consumer *uc) -{ - return __uprobe_register(inode, offset, ref_ctr_offset, uc); -} -EXPORT_SYMBOL_GPL(uprobe_register_refctr); - /* * uprobe_apply - unregister an already registered probe. * @inode: the file in which the probe has to be removed. diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index d1daeab1bbc1..ba62baec3152 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -3154,8 +3154,6 @@ struct bpf_uprobe_multi_link; struct bpf_uprobe { struct bpf_uprobe_multi_link *link; - loff_t offset; - unsigned long ref_ctr_offset; u64 cookie; struct uprobe_consumer consumer; }; @@ -3181,8 +3179,7 @@ static void bpf_uprobe_unregister(struct path *path, struct bpf_uprobe *uprobes, u32 i; for (i = 0; i < cnt; i++) { - uprobe_unregister(d_real_inode(path->dentry), uprobes[i].offset, - &uprobes[i].consumer); + uprobe_unregister(d_real_inode(path->dentry), &uprobes[i].consumer); } } @@ -3262,10 +3259,10 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link, for (i = 0; i < ucount; i++) { if (uoffsets && - put_user(umulti_link->uprobes[i].offset, uoffsets + i)) + put_user(umulti_link->uprobes[i].consumer.offset, uoffsets + i)) return -EFAULT; if (uref_ctr_offsets && - put_user(umulti_link->uprobes[i].ref_ctr_offset, uref_ctr_offsets + i)) + put_user(umulti_link->uprobes[i].consumer.ref_ctr_offset, uref_ctr_offsets + i)) return -EFAULT; if (ucookies && put_user(umulti_link->uprobes[i].cookie, ucookies + i)) @@ -3439,15 +3436,16 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr goto error_free; for (i = 0; i < cnt; i++) { - if (__get_user(uprobes[i].offset, uoffsets + i)) { + if (__get_user(uprobes[i].consumer.offset, uoffsets + i)) { err = -EFAULT; goto error_free; } - if (uprobes[i].offset < 0) { + if (uprobes[i].consumer.offset < 0) { err = -EINVAL; goto error_free; } - if (uref_ctr_offsets && __get_user(uprobes[i].ref_ctr_offset, uref_ctr_offsets + i)) { + if (uref_ctr_offsets && + __get_user(uprobes[i].consumer.ref_ctr_offset, uref_ctr_offsets + i)) { err = -EFAULT; goto error_free; } @@ -3477,10 +3475,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr &bpf_uprobe_multi_link_lops, prog); for (i = 0; i < cnt; i++) { - err = uprobe_register_refctr(d_real_inode(link->path.dentry), - uprobes[i].offset, - uprobes[i].ref_ctr_offset, - &uprobes[i].consumer); + err = uprobe_register(d_real_inode(link->path.dentry), &uprobes[i].consumer); if (err) { bpf_uprobe_unregister(&path, uprobes, i); goto error_free; diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index c98e3b3386ba..d786f99114be 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -60,8 +60,6 @@ struct trace_uprobe { struct path path; struct inode *inode; char *filename; - unsigned long offset; - unsigned long ref_ctr_offset; unsigned long nhit; struct trace_probe tp; }; @@ -205,7 +203,7 @@ static unsigned long translate_user_vaddr(unsigned long file_offset) udd = (void *) current->utask->vaddr; - base_addr = udd->bp_addr - udd->tu->offset; + base_addr = udd->bp_addr - udd->tu->consumer.offset; return base_addr + file_offset; } @@ -286,13 +284,13 @@ static bool trace_uprobe_match_command_head(struct trace_uprobe *tu, if (strncmp(tu->filename, argv[0], len) || argv[0][len] != ':') return false; - if (tu->ref_ctr_offset == 0) - snprintf(buf, sizeof(buf), "0x%0*lx", - (int)(sizeof(void *) * 2), tu->offset); + if (tu->consumer.ref_ctr_offset == 0) + snprintf(buf, sizeof(buf), "0x%0*llx", + (int)(sizeof(void *) * 2), tu->consumer.offset); else - snprintf(buf, sizeof(buf), "0x%0*lx(0x%lx)", - (int)(sizeof(void *) * 2), tu->offset, - tu->ref_ctr_offset); + snprintf(buf, sizeof(buf), "0x%0*llx(0x%llx)", + (int)(sizeof(void *) * 2), tu->consumer.offset, + tu->consumer.ref_ctr_offset); if (strcmp(buf, &argv[0][len + 1])) return false; @@ -410,7 +408,7 @@ static bool trace_uprobe_has_same_uprobe(struct trace_uprobe *orig, list_for_each_entry(orig, &tpe->probes, tp.list) { if (comp_inode != d_real_inode(orig->path.dentry) || - comp->offset != orig->offset) + comp->consumer.offset != orig->consumer.offset) continue; /* @@ -472,8 +470,8 @@ static int validate_ref_ctr_offset(struct trace_uprobe *new) for_each_trace_uprobe(tmp, pos) { if (new_inode == d_real_inode(tmp->path.dentry) && - new->offset == tmp->offset && - new->ref_ctr_offset != tmp->ref_ctr_offset) { + new->consumer.offset == tmp->consumer.offset && + new->consumer.ref_ctr_offset != tmp->consumer.ref_ctr_offset) { pr_warn("Reference counter offset mismatch."); return -EINVAL; } @@ -675,8 +673,8 @@ static int __trace_uprobe_create(int argc, const char **argv) WARN_ON_ONCE(ret != -ENOMEM); goto fail_address_parse; } - tu->offset = offset; - tu->ref_ctr_offset = ref_ctr_offset; + tu->consumer.offset = offset; + tu->consumer.ref_ctr_offset = ref_ctr_offset; tu->path = path; tu->filename = filename; @@ -746,12 +744,12 @@ static int trace_uprobe_show(struct seq_file *m, struct dyn_event *ev) char c = is_ret_probe(tu) ? 'r' : 'p'; int i; - seq_printf(m, "%c:%s/%s %s:0x%0*lx", c, trace_probe_group_name(&tu->tp), + seq_printf(m, "%c:%s/%s %s:0x%0*llx", c, trace_probe_group_name(&tu->tp), trace_probe_name(&tu->tp), tu->filename, - (int)(sizeof(void *) * 2), tu->offset); + (int)(sizeof(void *) * 2), tu->consumer.offset); - if (tu->ref_ctr_offset) - seq_printf(m, "(0x%lx)", tu->ref_ctr_offset); + if (tu->consumer.ref_ctr_offset) + seq_printf(m, "(0x%llx)", tu->consumer.ref_ctr_offset); for (i = 0; i < tu->tp.nr_args; i++) seq_printf(m, " %s=%s", tu->tp.args[i].name, tu->tp.args[i].comm); @@ -1089,12 +1087,7 @@ static int trace_uprobe_enable(struct trace_uprobe *tu, filter_func_t filter) tu->consumer.filter = filter; tu->inode = d_real_inode(tu->path.dentry); - if (tu->ref_ctr_offset) - ret = uprobe_register_refctr(tu->inode, tu->offset, - tu->ref_ctr_offset, &tu->consumer); - else - ret = uprobe_register(tu->inode, tu->offset, &tu->consumer); - + ret = uprobe_register(tu->inode, &tu->consumer); if (ret) tu->inode = NULL; @@ -1112,7 +1105,7 @@ static void __probe_event_disable(struct trace_probe *tp) if (!tu->inode) continue; - uprobe_unregister(tu->inode, tu->offset, &tu->consumer); + uprobe_unregister(tu->inode, &tu->consumer); tu->inode = NULL; } } @@ -1310,7 +1303,7 @@ static int uprobe_perf_close(struct trace_event_call *call, return 0; list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) { - ret = uprobe_apply(tu->inode, tu->offset, &tu->consumer, false); + ret = uprobe_apply(tu->inode, tu->consumer.offset, &tu->consumer, false); if (ret) break; } @@ -1334,7 +1327,7 @@ static int uprobe_perf_open(struct trace_event_call *call, return 0; list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) { - err = uprobe_apply(tu->inode, tu->offset, &tu->consumer, true); + err = uprobe_apply(tu->inode, tu->consumer.offset, &tu->consumer, true); if (err) { uprobe_perf_close(call, event); break; @@ -1464,7 +1457,7 @@ int bpf_get_uprobe_info(const struct perf_event *event, u32 *fd_type, *fd_type = is_ret_probe(tu) ? BPF_FD_TYPE_URETPROBE : BPF_FD_TYPE_UPROBE; *filename = tu->filename; - *probe_offset = tu->offset; + *probe_offset = tu->consumer.offset; *probe_addr = 0; return 0; } @@ -1627,9 +1620,9 @@ create_local_trace_uprobe(char *name, unsigned long offs, return ERR_CAST(tu); } - tu->offset = offs; + tu->consumer.offset = offs; tu->path = path; - tu->ref_ctr_offset = ref_ctr_offset; + tu->consumer.ref_ctr_offset = ref_ctr_offset; tu->filename = kstrdup(name, GFP_KERNEL); if (!tu->filename) { ret = -ENOMEM; diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c index b0132a342bb5..9ae2a3c64daa 100644 --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c @@ -377,7 +377,6 @@ uprobe_ret_handler(struct uprobe_consumer *self, unsigned long func, struct testmod_uprobe { struct path path; - loff_t offset; struct uprobe_consumer consumer; }; @@ -391,25 +390,24 @@ static int testmod_register_uprobe(loff_t offset) { int err = -EBUSY; - if (uprobe.offset) + if (uprobe.consumer.offset) return -EBUSY; mutex_lock(&testmod_uprobe_mutex); - if (uprobe.offset) + if (uprobe.consumer.offset) goto out; err = kern_path("/proc/self/exe", LOOKUP_FOLLOW, &uprobe.path); if (err) goto out; - err = uprobe_register_refctr(d_real_inode(uprobe.path.dentry), - offset, 0, &uprobe.consumer); - if (err) + uprobe.consumer.offset = offset; + err = uprobe_register(d_real_inode(uprobe.path.dentry), &uprobe.consumer); + if (err) { path_put(&uprobe.path); - else - uprobe.offset = offset; - + uprobe.consumer.offset = 0; + } out: mutex_unlock(&testmod_uprobe_mutex); return err; @@ -419,10 +417,9 @@ static void testmod_unregister_uprobe(void) { mutex_lock(&testmod_uprobe_mutex); - if (uprobe.offset) { - uprobe_unregister(d_real_inode(uprobe.path.dentry), - uprobe.offset, &uprobe.consumer); - uprobe.offset = 0; + if (uprobe.consumer.offset) { + uprobe_unregister(d_real_inode(uprobe.path.dentry), &uprobe.consumer); + uprobe.consumer.offset = 0; } mutex_unlock(&testmod_uprobe_mutex);
Simplify uprobe registration/unregistration interfaces by making offset and ref_ctr_offset part of uprobe_consumer "interface". In practice, all existing users already store these fields somewhere in uprobe_consumer's containing structure, so this doesn't pose any problem. We just move some fields around. On the other hand, this simplifies uprobe_register() and uprobe_unregister() API by having only struct uprobe_consumer as one thing representing attachment/detachment entity. This makes batched versions of uprobe_register() and uprobe_unregister() simpler. This also makes uprobe_register_refctr() unnecessary, so remove it and simplify consumers. No functional changes intended. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- include/linux/uprobes.h | 18 +++---- kernel/events/uprobes.c | 19 ++----- kernel/trace/bpf_trace.c | 21 +++----- kernel/trace/trace_uprobe.c | 53 ++++++++----------- .../selftests/bpf/bpf_testmod/bpf_testmod.c | 23 ++++---- 5 files changed, 55 insertions(+), 79 deletions(-)