linux-kernel - Re: [PATCH 6/6] regulator: core: simplify lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAE-0n53Zcxc0XFrSKmN1NS7RQHanPEBXy+KgJ3E_2jHaJBMevg@mail.gmail.com>
Date:   Wed, 30 Aug 2023 11:53:10 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Michał Mirosław <mirq-linux@...e.qmqm.pl>
Cc:     Doug Anderson <dianders@...omium.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
        Dmitry Osipenko <digetx@...il.com>
Subject: Re: [PATCH 6/6] regulator: core: simplify lock_two()

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.