[PATCH] rust: mutex: fix __mutex_init() usage in case of PREEMPT_RT

[PATCH] rust: mutex: fix __mutex_init() usage in case of PREEMPT_RT

[Dirk Behme]

In case CONFIG_PREEMPT_RT is enabled __mutex_init() becomes a macro
instead of an extern function (simplified from
include/linux/mutex.h):

extern void __mutex_init(struct mutex *lock, const char *name,
			 struct lock_class_key *key);
do {							\
	rt_mutex_base_init(&(mutex)->rtmutex);		\
	__mutex_rt_init((mutex), name, key);		\
} while (0)

The macro isn't resolved by bindgen, then. What results in a build
error:

error[E0425]: cannot find function `__mutex_init` in crate `bindings`
     --> rust/kernel/sync/lock/mutex.rs:104:28
      |
104   |           unsafe { bindings::__mutex_init(ptr, name, key) }
      |                              ^^^^^^^^^^^^ help: a function with a similar name exists: `__mutex_rt_init`
      |
     ::: rust/bindings/bindings_generated.rs:23722:5
      |
23722 | /     pub fn __mutex_rt_init(
23723 | |         lock: *mut mutex,
23724 | |         name: *const core::ffi::c_char,
23725 | |         key: *mut lock_class_key,
23726 | |     );
      | |_____- similarly named function `__mutex_rt_init` defined here

Fix this by adding a helper.

As explained by Gary Guo in [1] no #ifdef CONFIG_PREEMPT_RT
is needed here as rust/bindings/lib.rs prefers externed function to
helpers if an externed function exists.

Reported-by: Conor Dooley <conor@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/20240913-shack-estate-b376a65921b1@spud/
Link: https://lore.kernel.org/rust-for-linux/20240915123626.1a170103.gary@garyguo.net/ [1]
Fixes: 6d20d629c6d8 ("rust: lock: introduce `Mutex`")
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---

This is against rust-next to use the new helpers layout.
Due to the helpers layout change I'm thinking it is not worth
the effort to backport to older kernel versions, so I intentionally
did not include -stable.

 rust/helpers/mutex.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
index 200db7e6279f0..a17ca8cdb50ca 100644
--- a/rust/helpers/mutex.c
+++ b/rust/helpers/mutex.c
@@ -7,3 +7,9 @@ void rust_helper_mutex_lock(struct mutex *lock)
 {
 	mutex_lock(lock);
 }
+
+void rust_helper___mutex_init(struct mutex *mutex, const char *name,
+			      struct lock_class_key *key)
+{
+	__mutex_init(mutex, name, key);
+}
-- 
2.28.0

Re: [PATCH] rust: mutex: fix __mutex_init() usage in case of PREEMPT_RT

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

On Mon, Sep 16, 2024 at 09:37:52AM +0200, Dirk Behme wrote:
> In case CONFIG_PREEMPT_RT is enabled __mutex_init() becomes a macro
> instead of an extern function (simplified from
> include/linux/mutex.h):
> 
> extern void __mutex_init(struct mutex *lock, const char *name,
> 			 struct lock_class_key *key);
> do {							\
> 	rt_mutex_base_init(&(mutex)->rtmutex);		\
> 	__mutex_rt_init((mutex), name, key);		\
> } while (0)
> 
> The macro isn't resolved by bindgen, then. What results in a build
> error:
> 
> error[E0425]: cannot find function `__mutex_init` in crate `bindings`
>      --> rust/kernel/sync/lock/mutex.rs:104:28
>       |
> 104   |           unsafe { bindings::__mutex_init(ptr, name, key) }
>       |                              ^^^^^^^^^^^^ help: a function with a similar name exists: `__mutex_rt_init`
>       |
>      ::: rust/bindings/bindings_generated.rs:23722:5
>       |
> 23722 | /     pub fn __mutex_rt_init(
> 23723 | |         lock: *mut mutex,
> 23724 | |         name: *const core::ffi::c_char,
> 23725 | |         key: *mut lock_class_key,
> 23726 | |     );
>       | |_____- similarly named function `__mutex_rt_init` defined here
> 
> Fix this by adding a helper.
> 
> As explained by Gary Guo in [1] no #ifdef CONFIG_PREEMPT_RT
> is needed here as rust/bindings/lib.rs prefers externed function to
> helpers if an externed function exists.
> 
> Reported-by: Conor Dooley <conor@kernel.org>

Welp, once I turned off the 7 or so lock debugging config options again,
it does seem that this patch has solved the PREEMPT_RT related build
breakage - thanks.
Tested-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> Link: https://lore.kernel.org/rust-for-linux/20240913-shack-estate-b376a65921b1@spud/
> Link: https://lore.kernel.org/rust-for-linux/20240915123626.1a170103.gary@garyguo.net/ [1]
> Fixes: 6d20d629c6d8 ("rust: lock: introduce `Mutex`")
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] rust: mutex: fix __mutex_init() usage in case of PREEMPT_RT

[Gary Guo]

On Mon, 16 Sep 2024 09:37:52 +0200
Dirk Behme <dirk.behme@de.bosch.com> wrote:

> In case CONFIG_PREEMPT_RT is enabled __mutex_init() becomes a macro
> instead of an extern function (simplified from
> include/linux/mutex.h):
> 
> extern void __mutex_init(struct mutex *lock, const char *name,
> 			 struct lock_class_key *key);
> do {							\
> 	rt_mutex_base_init(&(mutex)->rtmutex);		\
> 	__mutex_rt_init((mutex), name, key);		\
> } while (0)
> 
> The macro isn't resolved by bindgen, then. What results in a build
> error:
> 
> error[E0425]: cannot find function `__mutex_init` in crate `bindings`
>      --> rust/kernel/sync/lock/mutex.rs:104:28  
>       |
> 104   |           unsafe { bindings::__mutex_init(ptr, name, key) }
>       |                              ^^^^^^^^^^^^ help: a function with a similar name exists: `__mutex_rt_init`
>       |
>      ::: rust/bindings/bindings_generated.rs:23722:5
>       |
> 23722 | /     pub fn __mutex_rt_init(
> 23723 | |         lock: *mut mutex,
> 23724 | |         name: *const core::ffi::c_char,
> 23725 | |         key: *mut lock_class_key,
> 23726 | |     );
>       | |_____- similarly named function `__mutex_rt_init` defined here
> 
> Fix this by adding a helper.
> 
> As explained by Gary Guo in [1] no #ifdef CONFIG_PREEMPT_RT
> is needed here as rust/bindings/lib.rs prefers externed function to
> helpers if an externed function exists.
> 
> Reported-by: Conor Dooley <conor@kernel.org>
> Link: https://lore.kernel.org/rust-for-linux/20240913-shack-estate-b376a65921b1@spud/
> Link: https://lore.kernel.org/rust-for-linux/20240915123626.1a170103.gary@garyguo.net/ [1]
> Fixes: 6d20d629c6d8 ("rust: lock: introduce `Mutex`")
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

Hi Dirk,

Reviewed-by: Gary Guo <gary@garyguo.net>

Thanks for sending the patch.

Best,
Gary

> ---
> 
> This is against rust-next to use the new helpers layout.
> Due to the helpers layout change I'm thinking it is not worth
> the effort to backport to older kernel versions, so I intentionally
> did not include -stable.
> 
>  rust/helpers/mutex.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
> index 200db7e6279f0..a17ca8cdb50ca 100644
> --- a/rust/helpers/mutex.c
> +++ b/rust/helpers/mutex.c
> @@ -7,3 +7,9 @@ void rust_helper_mutex_lock(struct mutex *lock)
>  {
>  	mutex_lock(lock);
>  }
> +
> +void rust_helper___mutex_init(struct mutex *mutex, const char *name,
> +			      struct lock_class_key *key)
> +{
> +	__mutex_init(mutex, name, key);
> +}

Re: [PATCH] rust: mutex: fix __mutex_init() usage in case of PREEMPT_RT

[Dirk Behme]

On 16.09.2024 21:28, Gary Guo wrote:
> On Mon, 16 Sep 2024 09:37:52 +0200
> Dirk Behme <dirk.behme@de.bosch.com> wrote:
> 
>> In case CONFIG_PREEMPT_RT is enabled __mutex_init() becomes a macro
>> instead of an extern function (simplified from
>> include/linux/mutex.h):
>>
>> extern void __mutex_init(struct mutex *lock, const char *name,
>> 			 struct lock_class_key *key);
>> do {							\
>> 	rt_mutex_base_init(&(mutex)->rtmutex);		\
>> 	__mutex_rt_init((mutex), name, key);		\
>> } while (0)


I just noticed that git commit fooled me and of course removed the 
C-macro lines beginning with '#' as git interprets that as comments :(

The above simplified example from mutex.h in the commit message should 
be correctly:

-- cut --
  #ifndef CONFIG_PREEMPT_RT
  extern void __mutex_init(struct mutex *lock, const char *name,
			 struct lock_class_key *key);
  #else
  #define __mutex_init(mutex, name, key)			\
  do {							\
	rt_mutex_base_init(&(mutex)->rtmutex);		\
	__mutex_rt_init((mutex), name, key);		\
  } while (0)
  #endif
-- cut --

Please let me know if you want a v2 with corrected commit message or if 
that could be corrected while applying :)

Best regards

Dirk

>> The macro isn't resolved by bindgen, then. What results in a build
>> error:
>>
>> error[E0425]: cannot find function `__mutex_init` in crate `bindings`
>>       --> rust/kernel/sync/lock/mutex.rs:104:28
>>        |
>> 104   |           unsafe { bindings::__mutex_init(ptr, name, key) }
>>        |                              ^^^^^^^^^^^^ help: a function with a similar name exists: `__mutex_rt_init`
>>        |
>>       ::: rust/bindings/bindings_generated.rs:23722:5
>>        |
>> 23722 | /     pub fn __mutex_rt_init(
>> 23723 | |         lock: *mut mutex,
>> 23724 | |         name: *const core::ffi::c_char,
>> 23725 | |         key: *mut lock_class_key,
>> 23726 | |     );
>>        | |_____- similarly named function `__mutex_rt_init` defined here
>>
>> Fix this by adding a helper.
>>
>> As explained by Gary Guo in [1] no #ifdef CONFIG_PREEMPT_RT
>> is needed here as rust/bindings/lib.rs prefers externed function to
>> helpers if an externed function exists.
>>
>> Reported-by: Conor Dooley <conor@kernel.org>
>> Link: https://lore.kernel.org/rust-for-linux/20240913-shack-estate-b376a65921b1@spud/
>> Link: https://lore.kernel.org/rust-for-linux/20240915123626.1a170103.gary@garyguo.net/ [1]
>> Fixes: 6d20d629c6d8 ("rust: lock: introduce `Mutex`")
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> 
> Hi Dirk,
> 
> Reviewed-by: Gary Guo <gary@garyguo.net>
> 
> Thanks for sending the patch.
> 
> Best,
> Gary
> 
>> ---
>>
>> This is against rust-next to use the new helpers layout.
>> Due to the helpers layout change I'm thinking it is not worth
>> the effort to backport to older kernel versions, so I intentionally
>> did not include -stable.
>>
>>   rust/helpers/mutex.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
>> index 200db7e6279f0..a17ca8cdb50ca 100644
>> --- a/rust/helpers/mutex.c
>> +++ b/rust/helpers/mutex.c
>> @@ -7,3 +7,9 @@ void rust_helper_mutex_lock(struct mutex *lock)
>>   {
>>   	mutex_lock(lock);
>>   }
>> +
>> +void rust_helper___mutex_init(struct mutex *mutex, const char *name,
>> +			      struct lock_class_key *key)
>> +{
>> +	__mutex_init(mutex, name, key);
>> +}
> 
> 

-- 
======================================================================
Dirk Behme                      Robert Bosch Car Multimedia GmbH
                                 CM/ESO2
Phone: +49 5121 49-3274         Dirk Behme
Fax:   +49 711 811 5053274      PO Box 77 77 77
mailto:dirk.behme@de.bosch.com  D-31132 Hildesheim - Germany

Bosch Group, Car Multimedia (CM)
              Engineering SW Operating Systems 2 (ESO2)

Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe
Sitz: Hildesheim
Registergericht: Amtsgericht Hildesheim HRB 201334
Aufsichtsratsvorsitzender: Dr. Dirk Hoheisel
Geschäftsführung: Dr. Steffen Berns;
                   Dr. Sven Ost, Jörg Pollak, Dr. Walter Schirm
======================================================================

Re: [PATCH] rust: mutex: fix __mutex_init() usage in case of PREEMPT_RT

[Miguel Ojeda]

On Mon, Sep 16, 2024 at 9:38 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Fix this by adding a helper.
>
> As explained by Gary Guo in [1] no #ifdef CONFIG_PREEMPT_RT
> is needed here as rust/bindings/lib.rs prefers externed function to
> helpers if an externed function exists.
>
> Reported-by: Conor Dooley <conor@kernel.org>
> Link: https://lore.kernel.org/rust-for-linux/20240913-shack-estate-b376a65921b1@spud/
> Link: https://lore.kernel.org/rust-for-linux/20240915123626.1a170103.gary@garyguo.net/ [1]
> Fixes: 6d20d629c6d8 ("rust: lock: introduce `Mutex`")
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

Applied to `rust-fixes` -- thanks everyone!

[ Reworded to include the proper example by Dirk. - Miguel ]

Cheers,
Miguel