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

Michał Mirosław posted 7 patches 2 years, 3 months ago
[PATCH v2 5/7] regulator/core: regulator_lock_contended: wrap ww_mutex lock sequence restart
Posted by Michał Mirosław 2 years, 3 months ago
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@rere.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)
+{
+	ww_mutex_lock_slow(&rdev->mutex, ww_ctx);
+	rdev->ref_cnt++;
+	rdev->mutex_owner = current;
+}
+
 /**
  * regulator_lock - lock a single regulator
  * @rdev:		regulator source
@@ -218,9 +234,7 @@ static void regulator_lock_two(struct regulator_dev *rdev1,
 	while (true) {
 		regulator_unlock(held);
 
-		ww_mutex_lock_slow(&contended->mutex, ww_ctx);
-		contended->ref_cnt++;
-		contended->mutex_owner = current;
+		regulator_lock_contended(contended, ww_ctx);
 		swap(held, contended);
 		ret = regulator_lock_nested(contended, ww_ctx);
 
@@ -376,10 +390,8 @@ static void regulator_lock_dependent(struct regulator_dev *rdev,
 
 	do {
 		if (new_contended_rdev) {
-			ww_mutex_lock_slow(&new_contended_rdev->mutex, ww_ctx);
+			regulator_lock_contended(new_contended_rdev, ww_ctx);
 			old_contended_rdev = new_contended_rdev;
-			old_contended_rdev->ref_cnt++;
-			old_contended_rdev->mutex_owner = current;
 		}
 
 		err = regulator_lock_recursive(rdev,
@@ -6085,10 +6097,8 @@ static void regulator_summary_lock(struct ww_acquire_ctx *ww_ctx)
 
 	do {
 		if (new_contended_rdev) {
-			ww_mutex_lock_slow(&new_contended_rdev->mutex, ww_ctx);
+			regulator_lock_contended(new_contended_rdev, ww_ctx);
 			old_contended_rdev = new_contended_rdev;
-			old_contended_rdev->ref_cnt++;
-			old_contended_rdev->mutex_owner = current;
 		}
 
 		err = regulator_summary_lock_all(ww_ctx,
-- 
2.39.2

Re: [PATCH v2 5/7] regulator/core: regulator_lock_contended: wrap ww_mutex lock sequence restart
Posted by Doug Anderson 2 years, 3 months ago
Hi,

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