On Mon Jun 23, 2025 at 7:11 PM CEST, Boqun Feng wrote:
> On Mon, Jun 23, 2025 at 05:14:37PM +0200, Benno Lossin wrote:
>> On Mon Jun 23, 2025 at 4:47 PM CEST, Boqun Feng wrote:
>> > On Mon, Jun 23, 2025 at 03:44:58PM +0200, Benno Lossin wrote:
>> >> 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.
>>
>> I think it's doable if we have
>>
>> impl WwActiveCtx {
>
> I think you mean *WwAcquireCtx*
Oh yeah.
>> fn begin(&self) -> WwActiveCtx<'_, ()>;
>> }
>>
>> struct WwActiveCtx<'a, Locks> {
>> locks: Locks,
>
> This probably need to to be Result<Locks>, because we may detect
> -DEADLOCK in the middle.
>
> let (a, c, d) = ctx.begin()
> .lock(a)
> .lock(b) // <- `b` may be locked by someone else. So we should
> // drop `a` and switch `locks` to an `Err(_)`.
> .lock(c) // <- this should be a no-op if `locks` is an `Err(_)`.
> .finish();
Hmm, I thought that we would go for the `lock_slow_path` thing, but
maybe that's the wrong thing to do? Maybe `lock` should return a result?
I'd have to see the use-cases...
>> _ctx: PhantomData<&'a WwAcquireCtx>,
>
> We can still take a reference to WwAcquireCtx here I think.
Yeah we have to do that in order to call lock on the mutexes.
>> }
>>
>> impl<'a, Locks> WwActiveCtx<'a, Locks>
>> where
>> Locks: Tuple
>> {
>> fn lock<'b, T>(
>> self,
>> lock: &'b WwMutex<T>,
>> ) -> WwActiveCtx<'a, Locks::Append<WwMutexGuard<'b, T>>>;
>>
>> fn lock_with<'b, T>(
>> self,
>> get_lock: impl FnOnce(&Locks) -> &'b WwMutex<T>,
>> ) -> WwActiveCtx<'a, Locks::Append<WwMutexGuard<'b, T>>>;
>> // I'm not 100% sure that the lifetimes will work out...
>
> I think we can make the following work?
>
> impl<'a, Locks> WwActiveCtx<'a, Locks>
> where
> Locks: Tuple
> {
> fn lock_with<T>(
> self,
> get_lock: impl FnOnce(&Locks) -> &WmMutex<T>,
> ) -> WwActiveCtx<'a, Locks::Append<WmMutexGuard<'a, T>>
> }
>
> because with a `WwActiveCtx<'a, Locks>`, we can get a `&'a Locks`, which
> will give us a `&'a WmMutex<T>`, and should be able to give us a
> `WmMutexGuard<'a, T>`.
I think this is more restrictive, since this will require that the mutex
is (potentially) locked for `'a` (you can drop the guard before, but you
can't drop the mutex itself). So again concrete use-cases should inform
our choice here.
>> fn finish(self) -> Locks;
>> }
>>
>> trait Tuple {
>> type Append<T>;
>>
>> fn append<T>(self, value: T) -> Self::Append<T>;
>> }
>>
>
> `Tuple` is good enough for its own, if you could remember, we have some
> ideas about using things like this to consolidate multiple `RcuOld` so
> that we can do one `synchronize_rcu()` for `RcuOld`s.
Yeah that's true, feel free to make a patch or good-first-issue, I won't
have time to create a series.
>> impl Tuple for () {
>> type Append<T> = (T,);
>>
>> fn append<T>(self, value: T) -> Self::Append<T> {
>> (value,)
>> }
>> }
>>
>> impl<T1> Tuple for (T1,) {
>> type Append<T> = (T1, T);
>>
>> fn append<T>(self, value: T) -> Self::Append<T> {
>> (self.0, value,)
>> }
>> }
>>
>> impl<T1, T2> Tuple for (T1, T2) {
>> type Append<T> = (T1, T2, T);
>>
>> fn append<T>(self, value: T) -> Self::Append<T> {
>> (self.0, self.1, value,)
>> }
>> }
>>
>> /* these can easily be generated by a macro */
>>
>> > 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.
>>
>> Hmm what about adding this to the above idea?:
>>
>> impl<'a, Locks> WwActiveCtx<'a, Locks>
>> where
>> Locks: Tuple
>> {
>> fn custom<L2>(self, action: impl FnOnce(Locks) -> L2) ->
>> WwActiveCtx<'a, L2>;
>> }
>>
>> Then you can do:
>>
>> let (a, c, d) = ctx.begin()
>> .lock(a)
>> .lock(b)
>> .lock(c)
>> .custom(|(a, _, c)| (a, c))
>> .lock(d)
>> .finish();
>>
>
> Seems reasonable. But we still need to present this to the end user to
> see how much they like it. For ww_mutex I think the major user is DRM,
> so add them into Cc list.
Yeah let's see some use-cases :)
---
Cheers,
Benno
