[libevl][PATCH] lib/mutex: init recursion count after trylock via the core

[libevl][PATCH] lib/mutex: init recursion count after trylock via the core

[Luc Montier]

evl_trylock_mutex() takes the slow path to the core when try_lock()
returns -ENODATA, e.g. for a thread running in-band. Unlike
evl_timedlock_mutex(), it does not initialize gate.nesting once
EVL_MONIOC_TRYENTER succeeds.

For a recursive mutex, nesting then stays at 0 after the first lock
instead of 1. After N successful lock calls by the owner, nesting is
N - 1, so the (N - 1)th unlock releases the mutex and the last one
fails with -EPERM.

Set nesting to 1 after a successful TRYENTER, as evl_timedlock_mutex()
does after EVL_MONIOC_ENTER.

Signed-off-by: Luc Montier <luc.montier@etu.unistra.fr>
---
Hello,

This patch is against libevl r50-5 (e7dc1af, Jan 2025), the version
that matches my kernel (6.6.74, EVL ABI level 37, Raspberry Pi 5).
I could not test it on master, which requires ABI 47.

Reproducer: a recursive mutex, a thread running in-band when it calls
evl_trylock_mutex() three times, then three evl_unlock_mutex() calls.
The last unlock fails with -EPERM, and gate.nesting reads 0, 1, 2 after
the locks. With the patch it reads 1, 2, 3 and all unlocks succeed.

On master, this code now seems to live in lib/intrinsics/gate.c. Is
this already fixed there, and where would the fix belong?

I just want to if it is an issue from libevl or a misuse on my part.

Best regards,
Luc Montier


 lib/mutex.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/mutex.c b/lib/mutex.c
index 276e77f..832268f 100644
--- a/lib/mutex.c
+++ b/lib/mutex.c
@@ -332,6 +332,7 @@ int evl_lock_mutex(struct evl_mutex *mutex)
 
 int evl_trylock_mutex(struct evl_mutex *mutex)
 {
+	struct evl_monitor_state *gst;
 	int ret;
 
 	ret = try_lock(mutex);
@@ -341,7 +342,12 @@ int evl_trylock_mutex(struct evl_mutex *mutex)
 	do
 		ret = oob_ioctl(mutex->u.active.efd, EVL_MONIOC_TRYENTER);
 	while (ret && errno == EINTR);

+	if (ret == 0) {
+		gst = mutex->u.active.state;
+		gst->u.gate.nesting = 1;
+	}
+
 	return ret ? -errno : 0;
 }
 
-- 
2.34.1

Re: [libevl][PATCH] lib/mutex: init recursion count after trylock via the core

[Philippe Gerum]

Luc Montier <luc.montier@etu.unistra.fr> writes:

> evl_trylock_mutex() takes the slow path to the core when try_lock()
> returns -ENODATA, e.g. for a thread running in-band. Unlike
> evl_timedlock_mutex(), it does not initialize gate.nesting once
> EVL_MONIOC_TRYENTER succeeds.
>
> For a recursive mutex, nesting then stays at 0 after the first lock
> instead of 1. After N successful lock calls by the owner, nesting is
> N - 1, so the (N - 1)th unlock releases the mutex and the last one
> fails with -EPERM.
>
> Set nesting to 1 after a successful TRYENTER, as evl_timedlock_mutex()
> does after EVL_MONIOC_ENTER.
>
> Signed-off-by: Luc Montier <luc.montier@etu.unistra.fr>
> ---
> Hello,
>
> This patch is against libevl r50-5 (e7dc1af, Jan 2025), the version
> that matches my kernel (6.6.74, EVL ABI level 37, Raspberry Pi 5).
> I could not test it on master, which requires ABI 47.
>
> Reproducer: a recursive mutex, a thread running in-band when it calls
> evl_trylock_mutex() three times, then three evl_unlock_mutex() calls.
> The last unlock fails with -EPERM, and gate.nesting reads 0, 1, 2 after
> the locks. With the patch it reads 1, 2, 3 and all unlocks succeed.
>
> On master, this code now seems to live in lib/intrinsics/gate.c. Is
> this already fixed there, and where would the fix belong?
>
> I just want to if it is an issue from libevl or a misuse on my part.
>
> Best regards,
> Luc Montier
>
>
>  lib/mutex.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lib/mutex.c b/lib/mutex.c
> index 276e77f..832268f 100644
> --- a/lib/mutex.c
> +++ b/lib/mutex.c
> @@ -332,6 +332,7 @@ int evl_lock_mutex(struct evl_mutex *mutex)
>  
>  int evl_trylock_mutex(struct evl_mutex *mutex)
>  {
> +	struct evl_monitor_state *gst;
>  	int ret;
>  
>  	ret = try_lock(mutex);
> @@ -341,7 +342,12 @@ int evl_trylock_mutex(struct evl_mutex *mutex)
>  	do
>  		ret = oob_ioctl(mutex->u.active.efd, EVL_MONIOC_TRYENTER);
>  	while (ret && errno == EINTR);
>
> +	if (ret == 0) {
> +		gst = mutex->u.active.state;
> +		gst->u.gate.nesting = 1;
> +	}
> +
>  	return ret ? -errno : 0;
>  }

Thanks for looking into this. I don't know about libevl r50{-5}, since
we are not adding any patch sequence to libevl releases. This said, this
issue was already fixed by commit 189e496e (ABI 44).

Please note that ABI 37 is quite old and obsolete on many aspects. You
may want to resync to the current code.

-- 
Philippe.

Re: [libevl][PATCH] lib/mutex: init recursion count after trylock via the core

[Philippe Gerum]

Philippe Gerum <rpm@xenomai.org> writes:

> Luc Montier <luc.montier@etu.unistra.fr> writes:
>
>> evl_trylock_mutex() takes the slow path to the core when try_lock()
>> returns -ENODATA, e.g. for a thread running in-band. Unlike
>> evl_timedlock_mutex(), it does not initialize gate.nesting once
>> EVL_MONIOC_TRYENTER succeeds.
>>
>> For a recursive mutex, nesting then stays at 0 after the first lock
>> instead of 1. After N successful lock calls by the owner, nesting is
>> N - 1, so the (N - 1)th unlock releases the mutex and the last one
>> fails with -EPERM.
>>
>> Set nesting to 1 after a successful TRYENTER, as evl_timedlock_mutex()
>> does after EVL_MONIOC_ENTER.
>>
>> Signed-off-by: Luc Montier <luc.montier@etu.unistra.fr>
>> ---
>> Hello,
>>
>> This patch is against libevl r50-5 (e7dc1af, Jan 2025), the version
>> that matches my kernel (6.6.74, EVL ABI level 37, Raspberry Pi 5).
>> I could not test it on master, which requires ABI 47.
>>
>> Reproducer: a recursive mutex, a thread running in-band when it calls
>> evl_trylock_mutex() three times, then three evl_unlock_mutex() calls.
>> The last unlock fails with -EPERM, and gate.nesting reads 0, 1, 2 after
>> the locks. With the patch it reads 1, 2, 3 and all unlocks succeed.
>>
>> On master, this code now seems to live in lib/intrinsics/gate.c. Is
>> this already fixed there, and where would the fix belong?
>>
>> I just want to if it is an issue from libevl or a misuse on my part.
>>
>> Best regards,
>> Luc Montier
>>
>>
>>  lib/mutex.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/mutex.c b/lib/mutex.c
>> index 276e77f..832268f 100644
>> --- a/lib/mutex.c
>> +++ b/lib/mutex.c
>> @@ -332,6 +332,7 @@ int evl_lock_mutex(struct evl_mutex *mutex)
>>  
>>  int evl_trylock_mutex(struct evl_mutex *mutex)
>>  {
>> +	struct evl_monitor_state *gst;
>>  	int ret;
>>  
>>  	ret = try_lock(mutex);
>> @@ -341,7 +342,12 @@ int evl_trylock_mutex(struct evl_mutex *mutex)
>>  	do
>>  		ret = oob_ioctl(mutex->u.active.efd, EVL_MONIOC_TRYENTER);
>>  	while (ret && errno == EINTR);
>>
>> +	if (ret == 0) {
>> +		gst = mutex->u.active.state;
>> +		gst->u.gate.nesting = 1;
>> +	}
>> +
>>  	return ret ? -errno : 0;
>>  }
>
> Thanks for looking into this. I don't know about libevl r50{-5}, since
> we are not adding any patch sequence to libevl releases. This said, this
> issue was already fixed by commit 189e496e (ABI 44).
>

Note: this is a kernel-side commit in branch rebase/v6.12.y-cip-evl
available from all kernel releases we maintain as well.

-- 
Philippe.