Re: [PATCH v12 5/6] rust: ww_mutex: implement LockSet

["Gary Guo" <gary@garyguo.net>]

On Thu Oct 15, 2026 at 12:26 PM BST, Onur Özkan wrote:
> On Mon, 12 Oct 2026 13:02:15 +0100
> Gary Guo <gary@garyguo.net> wrote:
>
>> On Mon Oct 12, 2026 at 11:26 AM BST, Onur Özkan wrote:
>> > LockSet is a high-level and safe API built on top of
>> > ww_mutex which provides a safe and easy to use API
>> > while keeping the ww_mutex semantics.
>> >
>> > When EDEADLK is hit it drops all held locks and waits for the
>> > contended mutex. It then retries the given (by the user)
>> > locking algorithm with that mutex held and the same acquire
>> > context until it succeeds.
>> 
>> Do we really need this, or this can be achieved by simple retry loops?
>> 
>>     fn retry_if_deadlock<R>(f: impl FnOnce() -> Result<R>) -> Result<R> {
>>         loop {
>>             match f() {
>>                 Err(EDEADLK) => (),
>>                 e => return e,
>>             }
>>         }
>>     }
>> 
>>     stack_pin_init!(let acq_ctx = AcquireCtx::new(&class));
>>     retry_if_deadlock(|| {
>>         let mut gaurd1 = acq_ctx.lock(&mutex1)?;
>>         let mut guard2 = acq_ctx.lock(&mutex2)?;
>> 
>>         acq_ctx.done();
>> 
>>         *guard1 += 1;
>>         *guard2 += 1;
>>     });
>> 
>> I think the only difference is that the `LockSet` acquires the contending mutex
>> first. However, I imagine we can solve this by having a wrapped `AcquireCtx`
>> that provides that.
>
> That's what LockSet already is. How would you design that wrapped AcquireCtx and
> what would we gain by replacing LockSet with another wrapper?

The LockSet API have multiple issues that I consider problematic:

* It always allocate, even if you don't need a dynamic set.

* Accessing data searches through the vec. To access all data, you will have
  O(n^2) complexity spent just on vec traversal.
 
* There's no way of getting multiple mutable borrow of the distinct locks.

My example above has none of the issue.

If you do not need the "acquire the contended lock first" behaviour, then
there's no need for wrapper, even. Also, if this behaviour is truly universal,
perhaps it should become part of `AcquireCtx`'s default behaviour instead.

>
> You need to remember the contended mutex, release all acquired locks before
> waiting for it and retry with that mutex held and the same acquire context. 

My example above doesn't have these issues. Releasing acquired locks if free
with `?`. Retrying is done by the helper function and not handled by user.
Acquire context if really easy to be misued, can be done by the helper function
too:
 
    fn ww_transaction<R>(
        class: &Class, f: impl FnOnce(&AcquireCtx) -> Result<R>
    ) -> Result<R> {
        stack_pin_init!(let acq_ctx = AcquireCtx::new(&class));
        loop {
            match f(&acq_Ctx) {
                Err(EDEADLK) => (),
                e => return e,
            }
        }
    }

    ww_transaction(&class, |acq_ctx| {
        let mut guard1 = acq_ctx.lock(&mutex1)?;
        let mut guard2 = acq_ctx.lock(&mutex2)?;

        acq_ctx.done();

        *guard1 += 1;
        *guard2 += 1;
    })


> Also for a dynamic set, you also need to keep the acquired locks held across loop
> iterations and helper calls and handle repeated objects. With ordinary guards,
> that bookkeeping falls on the caller unless the wrapper handles it. LockSet
> already does all this.

You can make dynamic lock set just a data structure and not do everything at
once, something like:

    ww_transaction(&class, |acq_ctx| {
        let dynset = DynLockSet::new()?;

        // Check if already inside the set, if not, lock it.
        dynset.lock(acq_ctx, &mutex1)?;
        dynset.lock(acq_ctx, &mutex2)?;

        acq_ctx.done();

        dynset.with_locked(&mutex1, |guard1| *guard1 += 1)?;
        dynset.with_locked(&mutex2, |guard2| *guard2 += 1)?;
    })

Best,
Gary