Re: [PATCH v3] reset: core: Serialize concurrent deassert/reset of shared resets

[Philipp Zabel]

On Mi, 2026-10-21 at 16:16 +0200, Alvin Šipraga wrote:
[...]
> Looking forward a bit to the second phase where we have everything
> serialized, maybe it will make sense to also lock around the call to the
> .status driver op too? Not suggesting we do it here (as this is just a
> bugfix), but in general I like when subsystems offer a guarantee that
> driver ops can't be called concurrently. Also the status is kind of
> undefined if you're currently in the process of manipulating the reset
> line.

That doesn't change with locking if you can't act on the result of the
status return value under the lock.

A concurrent assert/deassert may lose the race against status and
toggle the reset line right after status unlocks, before it returns.

> What I had in mind later on was to rename what Shengjiu has called
> shared_lock to ops_lock instead.

We could also combine this with the exclusive reset control's acquire
"lock" if we move that inside the SRCU section.

> With the following primitives:
> 
>   can_sleep?    CONTROL TYPE       LOCKING PRIMITIVE
>   --------------------------------------------------
>   no            shared             spin_lock_irqsave
>   yes           shared             mutex
>   no            exclusive          none? see [*]
>   yes           exclusive          none? see [*]

Yes.

> Where all reset control operation functions look like:
> 
>   reset_control_op() {
>     ...
>   
>     lock(ops_lock);
>   
>     __reset_control_op(); /* inc/dec, ->op() call, etc. here */
>   
>     unlock(ops_lock);
>   }

It might make sense to split these into _exclusive and _shared
variants.

> [*] I'm not sure if it makes sense to add any locking here? I can only
> see it as API misuse if somebody is manipulating an exclusive reset
> control from two places at once.

Right.


regards
Philipp