Message-Id: <DBOPIJHY9NZ7.2CU5XP7UY7ES3@kernel.org>
Date: Tue, 29 Jul 2025 19:15:12 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: Onur Özkan <work@...rozkan.dev>,
 <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Cc: <ojeda@...nel.org>, <alex.gaynor@...il.com>, <boqun.feng@...il.com>,
 <gary@...yguo.net>, <a.hindborg@...nel.org>, <aliceryhl@...gle.com>,
 <tmgross@...ch.edu>, <dakr@...nel.org>, <peterz@...radead.org>,
 <mingo@...hat.com>, <will@...nel.org>, <longman@...hat.com>,
 <felipe_life@...e.com>, <daniel@...lak.dev>, <bjorn3_gh@...tonmail.com>,
 "Lyude" <thatslyude@...il.com>
Subject: Re: [PATCH v5 0/3] rust: add `ww_mutex` support

On Thu Jul 24, 2025 at 3:53 PM CEST, Onur Özkan wrote:
> Hi again,
>
> Just finished going over the C-side use of `ww_mutex` today and I
> wanted to share some notes and thoughts based on that.

Thanks!

> To get the full context, you might want to take a look at this thread
> [1].
>
> - The first note I took is that we shouldn't allow locking without
> `WwAcquireCtx` (which is currently possible in v5). As explained in
> ww_mutex documentation [2], this basically turns it into a regular
> mutex and you don't get benefits of `ww_mutex`.
>
>  From what I have seen on the C side, there is no real use-case for
>  this. It doesn't make much sense to use `ww_mutex` just for
>  single-locking scenarios. Unless a specific use-case comes up, I think
>  we shouldn't support using it that way. I am planning to move the
>  `lock*` functions under `impl WwAcquireCtx` (as we discussed in [1]),
>  which will make `WwAcquireCtx` required by design and also simplify
>  the implementation a lot.

Sounds good to me. Although [2] states that:

    * Functions to only acquire a single w/w mutex, which results in the exact same
      semantics as a normal mutex. This is done by calling ww_mutex_lock with a NULL
      context.
    
      Again this is not strictly required. But often you only want to acquire a
      single lock in which case it's pointless to set up an acquire context (and so
      better to avoid grabbing a deadlock avoidance ticket).

So maybe it is needed? Would need some use-cases to determine this.

> - The second note is about how EDEADLK is handled. On the C side, it
> looks like some code paths may not release all the previously locked
> mutexes or have a special/custom logic when locking returns EDEADLK
> (see [3]). So, handling EDEADLK automatically (pointed
> in [1]) can be quite useful for most cases, but that could also be a
> limitation in certain scenarios.
>
>  I was thinking we could provide an alternative version of each `lock*`
>  function that accepts a closure which is called on the EDEADLK error.
>  This way, we can support both auto-release locks and custom logic for
>  handling EDEADLK scenarios.
>
>  Something like this (just a dummy code for demonstration):
>
>     ctx.lock_and_handle_edeadlk(|active_locks| {
>         // user-defined handling here
>     });

But this function wouldn't be locking any additional locks, right?

I think the closure makes sense to give as a way to allow custom code.
But we definitely should try to get the common use-cases closure-free
(except of course they run completely custom code to their specific
use-case).

We can also try to invent a custom return type that is used instead of
`Result`. So for example:

    let a: WwMutex<'_, A>;
    let b: WwMutex<'_, B>;
    let ctx: WwAcquireCtx<'_>;

    ctx.enter()             // EnteredContext<'_, ()>
        .lock(a)            // LockAttempt<'_, A, ()>
        .or_err(a)?         // EnteredContext<'_, (A,)>
        .lock(b)            // LockAttempt<'_, B, (A,)>
        .or_lock_slow(a, b) // Result<EnteredContext<'_, (A, B,)>>
        ?.finish()          // (WwMutexGuard<'_, A>, WwMutexGuard<'_, B>)

But no idea if this is actually useful...



What I think would be a good way forward would be to convert some
existing C uses of `WwMutex` to the intended Rust API and see how it
looks. Best to cover several different kinds of uses.

I quickly checked [2] and saw use-case with a dynamic number of locks
(all stored in a linked list). This isn't supported by the
`EnteredContext<'_, ()>` & tuple extenstion idea I had, so we need
something new for handling lists, graphs and other datastructures.

The example with the list also is a bit problematic from a guard point
of view, since we need a dynamic number of guards, which means we would
need to allocate...

[2]: https://www.kernel.org/doc/Documentation/locking/ww-mutex-design.txt

---
Cheers,
Benno

Powered by blists - more mailing lists