linux-kernel - Re: [PATCH v2 5/7] regulator/core: regulator_lock_contended: wrap ww

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD=FV=WXANhfLQfxYuuEWS_=ZcjR5LrmzAvXThrOECwqXhe5hw@mail.gmail.com>
Date:   Wed, 30 Aug 2023 13:21:51 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Michał Mirosław <mirq-linux@...e.qmqm.pl>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
        Stephen Boyd <swboyd@...omium.org>
Subject: Re: [PATCH v2 5/7] regulator/core: regulator_lock_contended: wrap
 ww_mutex lock sequence restart

Hi,

On Wed, Aug 30, 2023 at 10:35 AM Michał Mirosław
<mirq-linux@...e.qmqm.pl> wrote:
>
> Wrap locking a regulator after a failed ww_mutex locking sequence with a
> new function.  This is to deduplicate occurrences of the pattern and make
> it self-documenting.
>
> Signed-off-by: Michał Mirosław <mirq-linux@...e.qmqm.pl>
> ---
>  drivers/regulator/core.c | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index e89c12d27a9d..7201927c5d5b 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -154,6 +154,22 @@ static inline int regulator_lock_nested(struct regulator_dev *rdev,
>         return 0;
>  }
>
> +/**
> + * regulator_lock_contended - retry locking a regulator
> + * @rdev:              regulator source
> + * @ww_ctx:            w/w mutex acquire context
> + *
> + * Locks a regulator after a failed locking sequence (aborted
> + * with -EDEADLK).
> + */
> +static inline void regulator_lock_contended(struct regulator_dev *rdev,
> +                                           struct ww_acquire_ctx *ww_ctx)

nit: IMO "inline" should be reserved for places where it would be a
serious problem if the function wasn't inlined. For cases like this,
let the compiler do its job and decide whether we'll be better off
with the code inlined or not.

In any case:

Reviewed-by: Douglas Anderson <dianders@...omium.org>