LGTM, just a few notes...

On 07/31, Andrii Nakryiko wrote:
>
> @@ -59,6 +61,7 @@ struct uprobe {
>       struct list_head        pending_list;
>       struct uprobe_consumer  *consumers;
>       struct inode            *inode;         /* Also hold a ref to inode */
> +     struct rcu_head         rcu;

you can probably put the new member into the union with, say, rb_node.

> @@ -1945,9 +1950,14 @@ pre_ssout(struct uprobe *uprobe, struct pt_regs *regs, 
> unsigned long bp_vaddr)
>       if (!utask)
>               return -ENOMEM;
>
> +     if (!try_get_uprobe(uprobe))
> +             return -EINVAL;
> +

a bit off-topic right now, but it seems that we can simply kill
utask->active_uprobe. We can turn into into "bool has_active_uprobe"
and copy uprobe->arch into uprobe_task. Lets discuss this later.

> @@ -2201,13 +2215,15 @@ static void handle_swbp(struct pt_regs *regs)
>  {
>       struct uprobe *uprobe;
>       unsigned long bp_vaddr;
> -     int is_swbp;
> +     int is_swbp, srcu_idx;
>
>       bp_vaddr = uprobe_get_swbp_addr(regs);
>       if (bp_vaddr == uprobe_get_trampoline_vaddr())
>               return uprobe_handle_trampoline(regs);
>
> -     uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
> +     srcu_idx = srcu_read_lock(&uprobes_srcu);
> +
> +     uprobe = find_active_uprobe_rcu(bp_vaddr, &is_swbp);
>       if (!uprobe) {
>               if (is_swbp > 0) {
>                       /* No matching uprobe; signal SIGTRAP. */
> @@ -2223,6 +2239,7 @@ static void handle_swbp(struct pt_regs *regs)
>                        */
>                       instruction_pointer_set(regs, bp_vaddr);
>               }
> +             srcu_read_unlock(&uprobes_srcu, srcu_idx);
>               return;

Why not
                goto out;

?

Oleg.


Reply via email to