Re: [PATCH v5 2/3] implement ww_mutex abstraction for the Rust tree

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

On Mon, Jun 23, 2025 at 03:44:58PM +0200, Benno Lossin wrote:
> On Mon Jun 23, 2025 at 3:04 PM CEST, Boqun Feng wrote:
> > On Sun, Jun 22, 2025 at 11:18:24AM +0200, Benno Lossin wrote:
> >> On Sat Jun 21, 2025 at 8:44 PM CEST, Onur Özkan wrote:
> >> > Adds Rust bindings for the kernel's `ww_mutex` infrastructure to enable
> >> > deadlock-free acquisition of multiple related locks.
> >> >
> >> > The patch abstracts `ww_mutex.h` header and wraps the existing
> >> > C `ww_mutex` with three main types:
> >> >     - `WwClass` for grouping related mutexes
> >> >     - `WwAcquireCtx` for tracking lock acquisition context
> >> >     - `WwMutex<T>` for the actual lock
> >> 
> >> Going to repeat my question from the previous version:
> >> 
> >>     I don't know the design of `struct ww_mutex`, but from the code below I
> >>     gathered that it has some special error return values that signify that
> >>     one should release other locks.
> >>     
> >>     Did anyone think about making a more Rusty API that would allow one to
> >>     try to lock multiple mutexes at the same time (in a specified order) and
> >>     if it fails, it would do the resetting automatically?
> >
> > But the order may not be known ahead of time, for example say you have
> > a few:
> >
> >     pub struct Foo {
> >         other: Arc<WwMutex<Foo>>,
> > 	data: i32,
> >     }
> >
> > you need to get the lock of the current object in order to know what's
> > the next object to lock.
> >
> >> 
> >> I'm not familiar with ww_mutex, so I can't tell if there is something
> >> good that we could do.
> >> 
> >
> > It's not a bad idea when it can apply, but we still need to support the
> > case where the order is unknown.
> 
> I didn't have a concrete API in mind, but after having read the
> abstractions more, would this make sense?
> 
>     let ctx: &WwAcquireCtx = ...;
>     let m1: &WwMutex<T> = ...;
>     let m2: &WwMutex<Foo> = ...;
> 
>     let (t, foo, foo2) = ctx
>         .begin()
>         .lock(m1)
>         .lock(m2)
>         .lock_with(|(t, foo)| &*foo.other)
>         .finish();
> 

Cute!

However, each `.lock()` will need to be polymorphic over a tuple of
locks that are already held, right? Otherwise I don't see how
`.lock_with()` knows it's already held two locks. That sounds like a
challenge for implementation. We also need to take into consideration
that the user want to drop any lock in the sequence? E.g. the user
acquires a, b and c, and then drop b, and then acquires d. Which I think
is possible for ww_mutex.

Regards,
Boqun

>     let _: &mut T = t;
>     let _: &mut Foo = foo;
>     let _: &mut Foo = foo2;
> 
> ---
> Cheers,
> Benno