linux-kernel - Re: [PATCH] rust: Fix build error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zw2F27-Crx3G8_fz@Boquns-Mac-mini.local>
Date: Mon, 14 Oct 2024 13:58:03 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: Eder Zulian <ezulian@...hat.com>, Thomas Gleixner <tglx@...utronix.de>,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	williams@...hat.com, ojeda@...nel.org, alex.gaynor@...il.com,
	gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	a.hindborg@...nel.org, aliceryhl@...gle.com, tmgross@...ch.edu
Subject: Re: [PATCH] rust: Fix build error

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@...hat.com> wrote:
> >
> > Error observed while building a rt-debug kernel for aarch64.
> 
> Thanks for testing with Rust enabled!
> 
> > Suggested-by: Clark Williams <williams@...hat.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@...el.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