Re: [PATCH] rust: Fix build error

[Boqun Feng <boqun.feng@gmail.com>]

On Mon, Oct 14, 2024 at 10:38:45PM +0200, Miguel Ojeda wrote:
> On Mon, Oct 14, 2024 at 9:54 PM Eder Zulian <ezulian@redhat.com> wrote:
> >
> > Error observed while building a rt-debug kernel for aarch64.
> 
> Thanks for testing with Rust enabled!
> 
> > Suggested-by: Clark Williams <williams@redhat.com>
> 
> Do you mean `Reported-by`?
> 
> Also, I am not sure which `Fixes:` tag would fit best here, since
> `PREEMPT_RT` has been around for quite a while, but only enabled very
> recently. Thomas: do you have a preference?
> 
> In addition (sorry, it was in my backlog):
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202409251238.vetlgXE9-lkp@intel.com/
> 
> Finally, I think we should perhaps put a helper in `spinlock{_,rt}.h`
> that takes the `key` (instead of having this `#ifdef` here) and then
> just use that from the Rust helpers, because we don't want to
> duplicate such logic (conditionals) in helpers. And with the RT init
> open coding that Boqun mentioned, even more. After all, helpers are
> meant to be as straightforward as possible, and if we have this sort
> of thing in helpers, it is harder for everyone to keep them in sync.
> 

Make sense, and we did have something for !PREEMPT_RT spinlock:

	https://lore.kernel.org/rust-for-linux/20230411054543.21278-4-wedsonaf@gmail.com/

and we can do the same thing for PREEMPT_RT:

(untested code)

diff --git a/include/linux/spinlock_rt.h b/include/linux/spinlock_rt.h
index 61c49b16f69a..6ccdd2231575 100644
--- a/include/linux/spinlock_rt.h
+++ b/include/linux/spinlock_rt.h
@@ -16,20 +16,34 @@ static inline void __rt_spin_lock_init(spinlock_t *lock, const char *name,
 }
 #endif

+static inline void __spin_lock_init_with_key(spinlock_t *lock,
+                                            const char *name,
+                                            struct lock_class_key *key,
+                                            bool percpu)
+{
+       rt_mutex_base_init(&lock->lock);
+       __rt_spin_lock_init(slock, name, key, percpu);
+}
+
+static inline void spin_lock_init_with_key(spinlock_t *lock,
+                                            const char *name,
+                                            struct lock_class_key *key)
+{
+       __spin_lock_init_with_key(lock, name, key, false);
+}
+
 #define spin_lock_init(slock)                                  \
 do {                                                           \
        static struct lock_class_key __key;                     \
                                                                \
-       rt_mutex_base_init(&(slock)->lock);                     \
-       __rt_spin_lock_init(slock, #slock, &__key, false);      \
+       spin_lock_init_with_key(slock, #slock, &__key);         \
 } while (0)

 #define local_spin_lock_init(slock)                            \
 do {                                                           \
        static struct lock_class_key __key;                     \
                                                                \
-       rt_mutex_base_init(&(slock)->lock);                     \
-       __rt_spin_lock_init(slock, #slock, &__key, true);       \
+       __spin_lock_init_with_key(slock, #slock, &__key, true); \
 } while (0)

 extern void rt_spin_lock(spinlock_t *lock);

> In other words, I see helpers as following the same "avoid `#ifdef`s"
> rule that we prefer in C source files vs. headers.
> 
> What do you think, Thomas?
> 

Thanks for copying Thomas, my reply to Eder is here:

	https://lore.kernel.org/rust-for-linux/Zw1_rXUyjTBOh0QH@boqun-archlinux/

Regards,
Boqun

> >
> 
> Spurious newline.
> 
> Cheers,
> Miguel