Re: [PATCH v2 5/7] regulator/core: regulator_lock_contended: wrap ww_mutex lock sequence restart

From: Doug Anderson
Date: Wed Aug 30 2023 - 16:27:22 EST


Hi,

On Wed, Aug 30, 2023 at 10:35 AM Michał Mirosław
<mirq-linux@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxx>