On Thu, Aug 1, 2024 at 5:23 AM Liao, Chang <liaochang1@xxxxxxxxxx> wrote: > > > > 在 2024/8/1 5:42, Andrii Nakryiko 写道: > > To avoid unnecessarily taking a (brief) refcount on uprobe during > > breakpoint handling in handle_swbp for entry uprobes, make find_uprobe() > > not take refcount, but protect the lifetime of a uprobe instance with > > RCU. This improves scalability, as refcount gets quite expensive due to > > cache line bouncing between multiple CPUs. > > > > Specifically, we utilize our own uprobe-specific SRCU instance for this > > RCU protection. put_uprobe() will delay actual kfree() using call_srcu(). > > > > For now, uretprobe and single-stepping handling will still acquire > > refcount as necessary. We'll address these issues in follow up patches > > by making them use SRCU with timeout. > > > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > --- > > kernel/events/uprobes.c | 93 ++++++++++++++++++++++++----------------- > > 1 file changed, 55 insertions(+), 38 deletions(-) > > [...] > > > > @@ -2258,12 +2275,12 @@ static void handle_swbp(struct pt_regs *regs) > > if (arch_uprobe_skip_sstep(&uprobe->arch, regs)) > > goto out; > > > > - if (!pre_ssout(uprobe, regs, bp_vaddr)) > > - return; > > + if (pre_ssout(uprobe, regs, bp_vaddr)) > > + goto out; > > > > Regardless what pre_ssout() returns, it always reach the label 'out', so the > if block is unnecessary. yep, I know, but I felt like if (something is wrong) goto out; pattern was important to keep for each possible failing step for consistency. so unless this is a big deal, I'd keep it as is, as in the future there might be some other steps after pre_ssout() before returning, so this is a bit more "composable" > > > > - /* arch_uprobe_skip_sstep() succeeded, or restart if can't singlestep */ > > out: > > - put_uprobe(uprobe); > > + /* arch_uprobe_skip_sstep() succeeded, or restart if can't singlestep */ > > + srcu_read_unlock(&uprobes_srcu, srcu_idx); > > } > > > > /* > > -- > BR > Liao, Chang