Re: [PATCH 6/6] regulator: core: simplify lock_two()

[Stephen Boyd]

Quoting Michał Mirosław (2023-08-30 10:25:22)
>
> I see that you prefer the held/contended intermediary names. I'd like to
> understand why we differ in the perception of readability of this part
> of code so that the change is needed? The algorithm is simple enough,
> and it would work even if the locks weren't swapped for each iteration
> (even if slower to finish).

The locks need to be swapped per the documentation above
ww_mutex_lock_slow(). This is how we ensure forward progress.

> What is missing in the context of the
> function's code? Are there some assumptions not easily visible?

Yes, there are assumptions that are harder to keep track of with the
names 'rdev1' and 'rdev2'. We have to make sure that we call
ww_mutex_lock_slow() on the contended mutex, not the one that we could
get first. I find it easier to keep track of which regulator is
contended and which regulator is held by having the local variable
names. It's not up to me though as I'm not the maintainer here.

>
> BTW, I went on to add the regulator_lock_contended() to see how it
> would look. I'll send a v2 with it so we can apply your proposal if
> needed on top.

Got it.