On Wed, Jun 05, 2024 at 06:36:25PM +0200, Oleg Nesterov wrote:

On 06/05, Oleg Nesterov wrote:

quoted

On 06/05, Oleg Nesterov wrote:

quoted

quoted

+/*
+ * Make sure all the uprobe consumers have only one type of entry
+ * callback registered (either handler or handler_session) due to
+ * different return value actions.
+ */
+static int consumer_check(struct uprobe_consumer *curr, struct uprobe_consumer *uc)
+{
+	if (!curr)
+		return 0;
+	if (curr->handler_session || uc->handler_session)
+		return -EBUSY;
+	return 0;
+}

Hmm, I don't understand this code, it doesn't match the comment...

The comment says "all the uprobe consumers have only one type" but
consumer_check() will always fail if the the 1st or 2nd consumer has
->handler_session != NULL ?

Perhaps you meant

	if (!!curr->handler != !!uc->handler)
		return -EBUSY;

?

OK, the changelog says

	Which means that there can be only single user of a uprobe (inode +
	offset) when session consumer is registered to it.

so the code is correct. But I still think the comment is misleading.

Cough... perhaps it is correct but I am still confused even we forget about
the comment ;)

OK, uprobe can have a single consumer with ->handler_session != NULL. I guess
this is because return_instance->data is "global".

So uprobe can have multiple handler_session == NULL consumers before
handler_session != NULL, but not after ?

ah yea it should have done what's in the comment, so it's missing
the check for handler.. session handlers are meant to be exclusive

thanks,
jirka