uprobes: Add unique uprobe

[PATCH perf/core 01/11] uprobes: Add unique flag to uprobe consumer

Posted by Jiri Olsa 1 month ago

Adding unique flag to uprobe consumer to ensure it's the only consumer
attached on the uprobe.

This is helpful for use cases when consumer wants to change user space
registers, which might confuse other consumers. With this change we can
ensure there's only one consumer on specific uprobe.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/uprobes.h |  1 +
 kernel/events/uprobes.c | 30 ++++++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 08ef78439d0d..0df849dee720 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -60,6 +60,7 @@ struct uprobe_consumer {
 	struct list_head cons_node;
 
 	__u64 id;	/* set when uprobe_consumer is registered */
+	bool is_unique; /* the only consumer on uprobe */
 };
 
 #ifdef CONFIG_UPROBES
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 996a81080d56..b9b088f7333a 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1024,14 +1024,35 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset,
 	return uprobe;
 }
 
-static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
+static bool consumer_can_add(struct list_head *head, struct uprobe_consumer *uc)
+{
+	/* Uprobe has no consumer, we can add any. */
+	if (list_empty(head))
+		return true;
+	/* Uprobe has consumer/s, we can't add unique one. */
+	if (uc->is_unique)
+		return false;
+	/*
+	 * Uprobe has consumer/s, we can add nother consumer only if the
+	 * current consumer is not unique.
+	 **/
+	return !list_first_entry(head, struct uprobe_consumer, cons_node)->is_unique;
+}
+
+static int consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
 {
 	static atomic64_t id;
+	int ret = -EBUSY;
 
 	down_write(&uprobe->consumer_rwsem);
+	if (!consumer_can_add(&uprobe->consumers, uc))
+		goto unlock;
 	list_add_rcu(&uc->cons_node, &uprobe->consumers);
 	uc->id = (__u64) atomic64_inc_return(&id);
+	ret = 0;
+unlock:
 	up_write(&uprobe->consumer_rwsem);
+	return ret;
 }
 
 /*
@@ -1420,7 +1441,12 @@ struct uprobe *uprobe_register(struct inode *inode,
 		return uprobe;
 
 	down_write(&uprobe->register_rwsem);
-	consumer_add(uprobe, uc);
+	ret = consumer_add(uprobe, uc);
+	if (ret) {
+		put_uprobe(uprobe);
+		up_write(&uprobe->register_rwsem);
+		return ERR_PTR(ret);
+	}
 	ret = register_for_each_vma(uprobe, uc);
 	up_write(&uprobe->register_rwsem);
 
-- 
2.51.0

Re: [PATCH perf/core 01/11] uprobes: Add unique flag to uprobe consumer

Posted by Oleg Nesterov 4 weeks, 1 day ago

On 09/02, Jiri Olsa wrote:
>
> +static bool consumer_can_add(struct list_head *head, struct uprobe_consumer *uc)
> +{
> +	/* Uprobe has no consumer, we can add any. */
> +	if (list_empty(head))
> +		return true;
> +	/* Uprobe has consumer/s, we can't add unique one. */
> +	if (uc->is_unique)
> +		return false;
> +	/*
> +	 * Uprobe has consumer/s, we can add nother consumer only if the
> +	 * current consumer is not unique.
> +	 **/
> +	return !list_first_entry(head, struct uprobe_consumer, cons_node)->is_unique;
> +}

Since you are going to send V2 anyway... purely cosmetic and subjective nit,
but somehow I can't resist,

	bool consumer_can_add(struct list_head *head, struct uprobe_consumer *new)
	{
		struct uprobe_consumer *old = list_first_entry_or_null(...);

		return !old || (!old->exclusive && !new->exclusive);
	}

looks a bit more readable to me. Please ignore if you like your version more.

Oleg.

Re: [PATCH perf/core 01/11] uprobes: Add unique flag to uprobe consumer

Posted by Jiri Olsa 4 weeks, 1 day ago

On Wed, Sep 03, 2025 at 12:49:33PM +0200, Oleg Nesterov wrote:
> On 09/02, Jiri Olsa wrote:
> >
> > +static bool consumer_can_add(struct list_head *head, struct uprobe_consumer *uc)
> > +{
> > +	/* Uprobe has no consumer, we can add any. */
> > +	if (list_empty(head))
> > +		return true;
> > +	/* Uprobe has consumer/s, we can't add unique one. */
> > +	if (uc->is_unique)
> > +		return false;
> > +	/*
> > +	 * Uprobe has consumer/s, we can add nother consumer only if the
> > +	 * current consumer is not unique.
> > +	 **/
> > +	return !list_first_entry(head, struct uprobe_consumer, cons_node)->is_unique;
> > +}
> 
> Since you are going to send V2 anyway... purely cosmetic and subjective nit,
> but somehow I can't resist,
> 
> 	bool consumer_can_add(struct list_head *head, struct uprobe_consumer *new)
> 	{
> 		struct uprobe_consumer *old = list_first_entry_or_null(...);
> 
> 		return !old || (!old->exclusive && !new->exclusive);
> 	}
> 
> looks a bit more readable to me. Please ignore if you like your version more.

yep, looks better, thanks

jirka

Re: [PATCH perf/core 01/11] uprobes: Add unique flag to uprobe consumer

Posted by Masami Hiramatsu (Google) 1 month ago

On Tue,  2 Sep 2025 16:34:54 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> Adding unique flag to uprobe consumer to ensure it's the only consumer
> attached on the uprobe.
> 
> This is helpful for use cases when consumer wants to change user space
> registers, which might confuse other consumers. With this change we can
> ensure there's only one consumer on specific uprobe.

nit: Does this mean one callback (consumer) is exclusively attached?
If so, "exclusive" will be better wording?

The logic looks good to me.

Thanks,

> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/linux/uprobes.h |  1 +
>  kernel/events/uprobes.c | 30 ++++++++++++++++++++++++++++--
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
> index 08ef78439d0d..0df849dee720 100644
> --- a/include/linux/uprobes.h
> +++ b/include/linux/uprobes.h
> @@ -60,6 +60,7 @@ struct uprobe_consumer {
>  	struct list_head cons_node;
>  
>  	__u64 id;	/* set when uprobe_consumer is registered */
> +	bool is_unique; /* the only consumer on uprobe */
>  };
>  
>  #ifdef CONFIG_UPROBES
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 996a81080d56..b9b088f7333a 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1024,14 +1024,35 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset,
>  	return uprobe;
>  }
>  
> -static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
> +static bool consumer_can_add(struct list_head *head, struct uprobe_consumer *uc)
> +{
> +	/* Uprobe has no consumer, we can add any. */
> +	if (list_empty(head))
> +		return true;
> +	/* Uprobe has consumer/s, we can't add unique one. */
> +	if (uc->is_unique)
> +		return false;
> +	/*
> +	 * Uprobe has consumer/s, we can add nother consumer only if the
> +	 * current consumer is not unique.
> +	 **/
> +	return !list_first_entry(head, struct uprobe_consumer, cons_node)->is_unique;
> +}
> +
> +static int consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
>  {
>  	static atomic64_t id;
> +	int ret = -EBUSY;
>  
>  	down_write(&uprobe->consumer_rwsem);
> +	if (!consumer_can_add(&uprobe->consumers, uc))
> +		goto unlock;
>  	list_add_rcu(&uc->cons_node, &uprobe->consumers);
>  	uc->id = (__u64) atomic64_inc_return(&id);
> +	ret = 0;
> +unlock:
>  	up_write(&uprobe->consumer_rwsem);
> +	return ret;
>  }
>  
>  /*
> @@ -1420,7 +1441,12 @@ struct uprobe *uprobe_register(struct inode *inode,
>  		return uprobe;
>  
>  	down_write(&uprobe->register_rwsem);
> -	consumer_add(uprobe, uc);
> +	ret = consumer_add(uprobe, uc);
> +	if (ret) {
> +		put_uprobe(uprobe);
> +		up_write(&uprobe->register_rwsem);
> +		return ERR_PTR(ret);
> +	}
>  	ret = register_for_each_vma(uprobe, uc);
>  	up_write(&uprobe->register_rwsem);
>  
> -- 
> 2.51.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH perf/core 01/11] uprobes: Add unique flag to uprobe consumer

Posted by Jiri Olsa 1 month ago

On Wed, Sep 03, 2025 at 12:11:33AM +0900, Masami Hiramatsu wrote:
> On Tue,  2 Sep 2025 16:34:54 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
> 
> > Adding unique flag to uprobe consumer to ensure it's the only consumer
> > attached on the uprobe.
> > 
> > This is helpful for use cases when consumer wants to change user space
> > registers, which might confuse other consumers. With this change we can
> > ensure there's only one consumer on specific uprobe.
> 
> nit: Does this mean one callback (consumer) is exclusively attached?
> If so, "exclusive" will be better wording?

yes, exclusive is better, will change

thanks,
jirka

> 
> The logic looks good to me.
> 
> Thanks,
> 
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/linux/uprobes.h |  1 +
> >  kernel/events/uprobes.c | 30 ++++++++++++++++++++++++++++--
> >  2 files changed, 29 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
> > index 08ef78439d0d..0df849dee720 100644
> > --- a/include/linux/uprobes.h
> > +++ b/include/linux/uprobes.h
> > @@ -60,6 +60,7 @@ struct uprobe_consumer {
> >  	struct list_head cons_node;
> >  
> >  	__u64 id;	/* set when uprobe_consumer is registered */
> > +	bool is_unique; /* the only consumer on uprobe */
> >  };
> >  
> >  #ifdef CONFIG_UPROBES
> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> > index 996a81080d56..b9b088f7333a 100644
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -1024,14 +1024,35 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset,
> >  	return uprobe;
> >  }
> >  
> > -static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
> > +static bool consumer_can_add(struct list_head *head, struct uprobe_consumer *uc)
> > +{
> > +	/* Uprobe has no consumer, we can add any. */
> > +	if (list_empty(head))
> > +		return true;
> > +	/* Uprobe has consumer/s, we can't add unique one. */
> > +	if (uc->is_unique)
> > +		return false;
> > +	/*
> > +	 * Uprobe has consumer/s, we can add nother consumer only if the
> > +	 * current consumer is not unique.
> > +	 **/
> > +	return !list_first_entry(head, struct uprobe_consumer, cons_node)->is_unique;
> > +}
> > +
> > +static int consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
> >  {
> >  	static atomic64_t id;
> > +	int ret = -EBUSY;
> >  
> >  	down_write(&uprobe->consumer_rwsem);
> > +	if (!consumer_can_add(&uprobe->consumers, uc))
> > +		goto unlock;
> >  	list_add_rcu(&uc->cons_node, &uprobe->consumers);
> >  	uc->id = (__u64) atomic64_inc_return(&id);
> > +	ret = 0;
> > +unlock:
> >  	up_write(&uprobe->consumer_rwsem);
> > +	return ret;
> >  }
> >  
> >  /*
> > @@ -1420,7 +1441,12 @@ struct uprobe *uprobe_register(struct inode *inode,
> >  		return uprobe;
> >  
> >  	down_write(&uprobe->register_rwsem);
> > -	consumer_add(uprobe, uc);
> > +	ret = consumer_add(uprobe, uc);
> > +	if (ret) {
> > +		put_uprobe(uprobe);
> > +		up_write(&uprobe->register_rwsem);
> > +		return ERR_PTR(ret);
> > +	}
> >  	ret = register_for_each_vma(uprobe, uc);
> >  	up_write(&uprobe->register_rwsem);
> >  
> > -- 
> > 2.51.0
> > 
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>