Re: [PATCH v2 00/14] hrtimer Rust API

[Boqun Feng]

On Tue, Oct 01, 2024 at 02:37:46PM +0200, Dirk Behme wrote:
> On 18.09.2024 00:27, Andreas Hindborg wrote:
> > Hi!
> > 
> > This series adds support for using the `hrtimer` subsystem from Rust code.
> > 
> > I tried breaking up the code in some smaller patches, hopefully that will
> > ease the review process a bit.
> 
> Just fyi, having all 14 patches applied I get [1] on the first (doctest)
> Example from hrtimer.rs.
> 
> This is from lockdep:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/locking/lockdep.c#n4785
> 
> Having just a quick look I'm not sure what the root cause is. Maybe mutex in
> interrupt context? Or a more subtle one?

I think it's calling mutex inside an interrupt context as shown by the
callstack:

]  __mutex_lock+0xa0/0xa4
] ...
]  hrtimer_interrupt+0x1d4/0x2ac

, it is because:

+//! struct ArcIntrusiveTimer {
+//!     #[pin]
+//!     timer: Timer<Self>,
+//!     #[pin]
+//!     flag: Mutex<bool>,
+//!     #[pin]
+//!     cond: CondVar,
+//! }

has a Mutex<bool>, which actually should be a SpinLockIrq [1]. Note that
irq-off is needed for the lock, because otherwise we will hit a self
deadlock due to interrupts:

	spin_lock(&a);
	> timer interrupt
	  spin_lock(&a);

Also notice that the IrqDisabled<'_> token can be simply created by
::new(), because irq contexts should guarantee interrupt disabled (i.e.
we don't support nested interrupts*).

[*]: I vaguely remember we still have some driver code for slow devices
that will enable interrupts during an irq handler, but these are going
to be gone, we shouldn't really care about this in Rust code.

Regards,
Boqun

[1]: https://lore.kernel.org/rust-for-linux/20240916213025.477225-1-lyude@xxxxxxxxxx/


> 
> Best regards
> 
> Dirk
> 
> [1]
> 
>     # rust_doctest_kernel_hrtimer_rs_0.location: rust/kernel/hrtimer.rs:10
> rust_doctests_kernel: Timer called
> 
> =============================
> [ BUG: Invalid wait context ]
> 6.11.0-rc1-arm64 #28 Tainted: G                 N
> -----------------------------
> swapper/5/0 is trying to lock:
> ffff0004409ab900 (rust/doctests_kernel_generated.rs:1238){+.+.}-{3:3}, at:
> rust_helper_mutex_lock+0x10/0x18
> other info that might help us debug this:
> context-{2:2}
> no locks held by swapper/5/0.
> stack backtrace:
> CPU: 5 UID: 0 PID: 0 Comm: swapper/5 Tainted: G N 6.11.0-rc1-arm64 #28
> Tainted: [N]=TEST
> Hardware name: ARM64 based board (DT)
> Call trace:
>  $x.11+0x98/0xb4
>  show_stack+0x14/0x1c
>  $x.3+0x3c/0x94
>  dump_stack+0x14/0x1c
>  $x.205+0x538/0x594
>  $x.179+0xd0/0x18c
>  __mutex_lock+0xa0/0xa4
>  mutex_lock_nested+0x20/0x28
>  rust_helper_mutex_lock+0x10/0x18
> 
> _RNvXs_NvNvNvCslTRHJHclVGW_25doctests_kernel_generated32rust_doctest_kernel_hrtimer_rs_04main41__doctest_main_rust_kernel_hrtimer_rs_10_0NtB4_17ArcIntrusiveTimerNtNtCsclYTRz49wqv_6kernel7hrtimer13TimerCallback3run+0x5c/0xd0
> 
> _RNvXs1_NtNtCsclYTRz49wqv_6kernel7hrtimer3arcINtNtNtB9_4sync3arc3ArcNtNvNvNvCslTRHJHclVGW_25doctests_kernel_generated32rust_doctest_kernel_hrtimer_rs_04main41__doctest_main_rust_kernel_hrtimer_rs_10_017ArcIntrusiveTimerENtB7_16RawTimerCallback3runB1b_+0x20/0x2c
>  $x.90+0x64/0x70
>  hrtimer_interrupt+0x1d4/0x2ac
>  arch_timer_handler_phys+0x34/0x40
>  $x.62+0x50/0x54
>  generic_handle_domain_irq+0x28/0x40
>  $x.154+0x58/0x6c
>  $x.471+0x10/0x20
>  el1_interrupt+0x70/0x94
>  el1h_64_irq_handler+0x14/0x1c
>  el1h_64_irq+0x64/0x68
>  arch_local_irq_enable+0x4/0x8
>  cpuidle_enter+0x34/0x48
>  $x.37+0x58/0xe4
>  cpu_startup_entry+0x30/0x34
>  $x.2+0xf8/0x118
>  $x.13+0x0/0x4
> rust_doctests_kernel: Timer called
> rust_doctests_kernel: Timer called
> rust_doctests_kernel: Timer called
> rust_doctests_kernel: Timer called
> rust_doctests_kernel: Counted to 5
>     ok 22 rust_doctest_kernel_hrtimer_rs_0
>     # rust_doctest_kernel_hrtimer_rs_1.location: rust/kernel/hrtimer.rs:137
> rust_doctests_kernel: Hello from the future
> rust_doctests_kernel: Flag raised
>     ok 23 rust_doctest_kernel_hrtimer_rs_1
>     # rust_doctest_kernel_hrtimer_rs_2.location: rust/kernel/hrtimer.rs:76
> rust_doctests_kernel: Timer called
> rust_doctests_kernel: Flag raised
>     ok 24 rust_doctest_kernel_hrtimer_rs_2