On 07/31, Andrii Nakryiko wrote:
>
> @@ -1120,17 +1098,19 @@ void uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *uc)
> int err;
>
> down_write(&uprobe->register_rwsem);
> - if (WARN_ON(!consumer_del(uprobe, uc))) {
> - err = -ENOENT;
OK, I agree, this should never happen.
But if you remove this check, then
> int uprobe_apply(struct uprobe *uprobe, struct uprobe_consumer *uc, bool add)
> {
> struct uprobe_consumer *con;
> - int ret = -ENOENT;
> + int ret = -ENOENT, srcu_idx;
>
> down_write(&uprobe->register_rwsem);
> - for (con = uprobe->consumers; con && con != uc ; con = con->next)
> - ;
> - if (con)
> - ret = register_for_each_vma(uprobe, add ? uc : NULL);
> +
> + srcu_idx = srcu_read_lock(&uprobes_srcu);
> + list_for_each_entry_srcu(con, &uprobe->consumers, cons_node,
> + srcu_read_lock_held(&uprobes_srcu)) {
> + if (con == uc) {
> + ret = register_for_each_vma(uprobe, add ? uc : NULL);
> + break;
> + }
> + }
we can probably remove the similar check above?
I mean, why do we need the list_for_each_entry_srcu() above? Is it possible
that uprobe_apply(uprobe, uc) is called when "uc" is not on the ->consumers
list?
At first glance I see no problems in this patch... but you know, my eyes are
already blurring, I'll continue tomorrow and read this patch again.
Oleg.
