diff mbox series

reset: Further simplify locking with guard()

Message ID 20240927-reset-guard-v1-1-293bf1302210@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series reset: Further simplify locking with guard() | expand

Commit Message

Philipp Zabel Sept. 27, 2024, 2:02 p.m. UTC 
Use guard(mutex) to automatically unlock mutexes when going out of
scope. Simplify error paths by removing a goto and manual mutex
unlocking in multiple places.

Follow-up to commit 3ec21e7fa854 ("reset: simplify locking with
guard()").

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/core.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)


---
base-commit: 487b1b32e317b85c2948eb4013f3e089a0433d49
change-id: 20240927-reset-guard-c42dfd2a26c7

Best regards,

Comments

Al Viro Sept. 28, 2024, 10:27 p.m. UTC | #1 
On Fri, Sep 27, 2024 at 04:02:32PM +0200, Philipp Zabel wrote:
> Use guard(mutex) to automatically unlock mutexes when going out of
> scope. Simplify error paths by removing a goto and manual mutex
> unlocking in multiple places.

And that, folks, is a live example of the reasons why guard() is an
attractive nuisance.  We really need a very loud warning on
cleanup.h stuff - otherwise such patches from well-meaning folks
will keep coming.

> @@ -1041,29 +1036,27 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
>  		}
>  	}
>  
> -	mutex_lock(&reset_list_mutex);
> +	guard(mutex)(&reset_list_mutex);
>  	rcdev = __reset_find_rcdev(&args, gpio_fallback);
>  	if (!rcdev) {
>  		rstc = ERR_PTR(-EPROBE_DEFER);
> -		goto out_unlock;
> +		goto out_put;
>  	}
>  
>  	if (WARN_ON(args.args_count != rcdev->of_reset_n_cells)) {
>  		rstc = ERR_PTR(-EINVAL);
> -		goto out_unlock;
> +		goto out_put;
>  	}
>  
>  	rstc_id = rcdev->of_xlate(rcdev, &args);
>  	if (rstc_id < 0) {
>  		rstc = ERR_PTR(rstc_id);
> -		goto out_unlock;
> +		goto out_put;
>  	}
>  
>  	/* reset_list_mutex also protects the rcdev's reset_control list */
>  	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
>  
> -out_unlock:
> -	mutex_unlock(&reset_list_mutex);
>  out_put:
>  	of_node_put(args.np);

Guess what happens if you take goto out_put prior to the entire thing,
in
                ret = __reset_add_reset_gpio_device(&args);
		if (ret) {
			rstc = ERR_PTR(ret);
			goto out_put;
		}
That patch adds implicit mutex_unlock() at the points where we leave
the scope.  Which extends to the end of function.  In other words, there is
one downstream of out_put, turning any goto out_put upstream of guard() into
a bug.

What's worse, that bug is not caught by gcc - it quietly generates bogus code
that will get unnoticed until we get an error from __reset_add_reset_gpio_device()
call.  At that point we'll look at the contents of uninitialized variable and,
if we are unlucky, call mutex_unlock() (with hell knows what pointer passed to it -
not that mutex_unlock(&reset_list_mutex) would do us any good at that point, since
we hadn't locked it in the first place).

Folks, that kind of cleanup patches is bloody dangerous; even something that
currently avoids that crap can easily grow that kind of quiet breakage later.
kernel test robot Sept. 29, 2024, 7:39 a.m. UTC | #2 
Hi Philipp,

kernel test robot noticed the following build errors:

[auto build test ERROR on 487b1b32e317b85c2948eb4013f3e089a0433d49]

url:    https://github.com/intel-lab-lkp/linux/commits/Philipp-Zabel/reset-Further-simplify-locking-with-guard/20240927-220355
base:   487b1b32e317b85c2948eb4013f3e089a0433d49
patch link:    https://lore.kernel.org/r/20240927-reset-guard-v1-1-293bf1302210%40pengutronix.de
patch subject: [PATCH] reset: Further simplify locking with guard()
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240929/202409291457.lc5Xgv3u-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240929/202409291457.lc5Xgv3u-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409291457.lc5Xgv3u-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/reset/core.c:1035:4: error: cannot jump from this goto statement to its label
    1035 |                         goto out_put;
         |                         ^
   drivers/reset/core.c:1039:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
    1039 |         guard(mutex)(&reset_list_mutex);
         |         ^
   include/linux/cleanup.h:167:15: note: expanded from macro 'guard'
     167 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:189:29: note: expanded from macro '__UNIQUE_ID'
     189 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:110:1: note: expanded from here
     110 | __UNIQUE_ID_guard501
         | ^
   1 error generated.


vim +1035 drivers/reset/core.c

c721f189e89c0d Krzysztof Kozlowski 2024-01-29   989  
1c5e05c23f4a64 Philipp Zabel       2021-03-04   990  struct reset_control *
1c5e05c23f4a64 Philipp Zabel       2021-03-04   991  __of_reset_control_get(struct device_node *node, const char *id, int index,
1c5e05c23f4a64 Philipp Zabel       2021-03-04   992  		       bool shared, bool optional, bool acquired)
61fc41317666be Philipp Zabel       2012-11-19   993  {
c721f189e89c0d Krzysztof Kozlowski 2024-01-29   994  	bool gpio_fallback = false;
d056c9b8191867 Philipp Zabel       2015-12-08   995  	struct reset_control *rstc;
c721f189e89c0d Krzysztof Kozlowski 2024-01-29   996  	struct reset_controller_dev *rcdev;
61fc41317666be Philipp Zabel       2012-11-19   997  	struct of_phandle_args args;
61fc41317666be Philipp Zabel       2012-11-19   998  	int rstc_id;
61fc41317666be Philipp Zabel       2012-11-19   999  	int ret;
61fc41317666be Philipp Zabel       2012-11-19  1000  
6c96f05c8bb8bc Hans de Goede       2016-02-23  1001  	if (!node)
6c96f05c8bb8bc Hans de Goede       2016-02-23  1002  		return ERR_PTR(-EINVAL);
6c96f05c8bb8bc Hans de Goede       2016-02-23  1003  
6c96f05c8bb8bc Hans de Goede       2016-02-23  1004  	if (id) {
6c96f05c8bb8bc Hans de Goede       2016-02-23  1005  		index = of_property_match_string(node,
6c96f05c8bb8bc Hans de Goede       2016-02-23  1006  						 "reset-names", id);
bb475230b8e59a Ramiro Oliveira     2017-01-13  1007  		if (index == -EILSEQ)
bb475230b8e59a Ramiro Oliveira     2017-01-13  1008  			return ERR_PTR(index);
6c96f05c8bb8bc Hans de Goede       2016-02-23  1009  		if (index < 0)
bb475230b8e59a Ramiro Oliveira     2017-01-13  1010  			return optional ? NULL : ERR_PTR(-ENOENT);
6c96f05c8bb8bc Hans de Goede       2016-02-23  1011  	}
6c96f05c8bb8bc Hans de Goede       2016-02-23  1012  
fc0a5921561c71 Maxime Ripard       2013-12-20  1013  	ret = of_parse_phandle_with_args(node, "resets", "#reset-cells",
61fc41317666be Philipp Zabel       2012-11-19  1014  					 index, &args);
bb475230b8e59a Ramiro Oliveira     2017-01-13  1015  	if (ret == -EINVAL)
61fc41317666be Philipp Zabel       2012-11-19  1016  		return ERR_PTR(ret);
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1017  	if (ret) {
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1018  		if (!IS_ENABLED(CONFIG_RESET_GPIO))
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1019  			return optional ? NULL : ERR_PTR(ret);
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1020  
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1021  		/*
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1022  		 * There can be only one reset-gpio for regular devices, so
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1023  		 * don't bother with the "reset-gpios" phandle index.
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1024  		 */
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1025  		ret = of_parse_phandle_with_args(node, "reset-gpios", "#gpio-cells",
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1026  						 0, &args);
bb475230b8e59a Ramiro Oliveira     2017-01-13  1027  		if (ret)
bb475230b8e59a Ramiro Oliveira     2017-01-13  1028  			return optional ? NULL : ERR_PTR(ret);
61fc41317666be Philipp Zabel       2012-11-19  1029  
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1030  		gpio_fallback = true;
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1031  
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1032  		ret = __reset_add_reset_gpio_device(&args);
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1033  		if (ret) {
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1034  			rstc = ERR_PTR(ret);
c721f189e89c0d Krzysztof Kozlowski 2024-01-29 @1035  			goto out_put;
61fc41317666be Philipp Zabel       2012-11-19  1036  		}
61fc41317666be Philipp Zabel       2012-11-19  1037  	}
61fc41317666be Philipp Zabel       2012-11-19  1038  
784c4fbce820c0 Philipp Zabel       2024-09-27  1039  	guard(mutex)(&reset_list_mutex);
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1040  	rcdev = __reset_find_rcdev(&args, gpio_fallback);
61fc41317666be Philipp Zabel       2012-11-19  1041  	if (!rcdev) {
b790c8ea5593d6 Geert Uytterhoeven  2018-10-08  1042  		rstc = ERR_PTR(-EPROBE_DEFER);
784c4fbce820c0 Philipp Zabel       2024-09-27  1043  		goto out_put;
61fc41317666be Philipp Zabel       2012-11-19  1044  	}
61fc41317666be Philipp Zabel       2012-11-19  1045  
e677774f502635 Maxime Ripard       2016-01-14  1046  	if (WARN_ON(args.args_count != rcdev->of_reset_n_cells)) {
b790c8ea5593d6 Geert Uytterhoeven  2018-10-08  1047  		rstc = ERR_PTR(-EINVAL);
784c4fbce820c0 Philipp Zabel       2024-09-27  1048  		goto out_put;
e677774f502635 Maxime Ripard       2016-01-14  1049  	}
e677774f502635 Maxime Ripard       2016-01-14  1050  
61fc41317666be Philipp Zabel       2012-11-19  1051  	rstc_id = rcdev->of_xlate(rcdev, &args);
61fc41317666be Philipp Zabel       2012-11-19  1052  	if (rstc_id < 0) {
b790c8ea5593d6 Geert Uytterhoeven  2018-10-08  1053  		rstc = ERR_PTR(rstc_id);
784c4fbce820c0 Philipp Zabel       2024-09-27  1054  		goto out_put;
61fc41317666be Philipp Zabel       2012-11-19  1055  	}
61fc41317666be Philipp Zabel       2012-11-19  1056  
c15ddec2ca0607 Hans de Goede       2016-02-23  1057  	/* reset_list_mutex also protects the rcdev's reset_control list */
c84b0326d5e4fe Philipp Zabel       2019-02-21  1058  	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
61fc41317666be Philipp Zabel       2012-11-19  1059  
c721f189e89c0d Krzysztof Kozlowski 2024-01-29  1060  out_put:
b790c8ea5593d6 Geert Uytterhoeven  2018-10-08  1061  	of_node_put(args.np);
61fc41317666be Philipp Zabel       2012-11-19  1062  
61fc41317666be Philipp Zabel       2012-11-19  1063  	return rstc;
61fc41317666be Philipp Zabel       2012-11-19  1064  }
6c96f05c8bb8bc Hans de Goede       2016-02-23  1065  EXPORT_SYMBOL_GPL(__of_reset_control_get);
61fc41317666be Philipp Zabel       2012-11-19  1066
Markus Elfring Sept. 29, 2024, 10:45 a.m. UTC | #3 
> Use guard(mutex) to automatically unlock mutexes when going out of
> scope. Simplify error paths by removing a goto and manual mutex
> unlocking in multiple places.
…
> +++ b/drivers/reset/core.c
…
@@ -1041,29 +1036,27 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
 		}
 	}

-	mutex_lock(&reset_list_mutex);
+	guard(mutex)(&reset_list_mutex);
 	rcdev = __reset_find_rcdev(&args, gpio_fallback);
…
 	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);

-out_unlock:
-	mutex_unlock(&reset_list_mutex);
 out_put:
 	of_node_put(args.np);
…

Would you like to preserve the same lock scope (which ended before this function call)?


@@ -1098,7 +1091,7 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 	const char *dev_id = dev_name(dev);
 	struct reset_control *rstc = NULL;

-	mutex_lock(&reset_lookup_mutex);
+	guard(mutex)(&reset_lookup_mutex);

 	list_for_each_entry(lookup, &reset_lookup_list, list) {
…
 			break;
 		}
 	}

-	mutex_unlock(&reset_lookup_mutex);
-
 	if (!rstc)
 		return optional ? NULL : ERR_PTR(-ENOENT);
…

Would you really like to increase the lock scope here?

Regards,
Markus
Krzysztof Kozlowski Sept. 29, 2024, 6:48 p.m. UTC | #4 
On 29/09/2024 00:27, Al Viro wrote:
> On Fri, Sep 27, 2024 at 04:02:32PM +0200, Philipp Zabel wrote:
>> Use guard(mutex) to automatically unlock mutexes when going out of
>> scope. Simplify error paths by removing a goto and manual mutex
>> unlocking in multiple places.
> 
> And that, folks, is a live example of the reasons why guard() is an
> attractive nuisance.  We really need a very loud warning on
> cleanup.h stuff - otherwise such patches from well-meaning folks
> will keep coming.
> 
>> @@ -1041,29 +1036,27 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
>>  		}
>>  	}
>>  
>> -	mutex_lock(&reset_list_mutex);
>> +	guard(mutex)(&reset_list_mutex);
>>  	rcdev = __reset_find_rcdev(&args, gpio_fallback);
>>  	if (!rcdev) {
>>  		rstc = ERR_PTR(-EPROBE_DEFER);
>> -		goto out_unlock;
>> +		goto out_put;
>>  	}
>>  
>>  	if (WARN_ON(args.args_count != rcdev->of_reset_n_cells)) {
>>  		rstc = ERR_PTR(-EINVAL);
>> -		goto out_unlock;
>> +		goto out_put;
>>  	}
>>  
>>  	rstc_id = rcdev->of_xlate(rcdev, &args);
>>  	if (rstc_id < 0) {
>>  		rstc = ERR_PTR(rstc_id);
>> -		goto out_unlock;
>> +		goto out_put;
>>  	}
>>  
>>  	/* reset_list_mutex also protects the rcdev's reset_control list */
>>  	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
>>  
>> -out_unlock:
>> -	mutex_unlock(&reset_list_mutex);
>>  out_put:
>>  	of_node_put(args.np);
> 
> Guess what happens if you take goto out_put prior to the entire thing,
> in
>                 ret = __reset_add_reset_gpio_device(&args);
> 		if (ret) {
> 			rstc = ERR_PTR(ret);
> 			goto out_put;
> 		}
> That patch adds implicit mutex_unlock() at the points where we leave
> the scope.  Which extends to the end of function.  In other words, there is
> one downstream of out_put, turning any goto out_put upstream of guard() into
> a bug.
> 

cleanup.h also mentions that one should do not mix cleanup with existing
goto, because of possibility of above issue.

But except careful review, this patch should have been simply compile
tested which would point to the issue above. Any guard/scope works must
be checked with clang W=1, which reports jumps over init.

Best regards,
Krzysztof
Philipp Zabel Sept. 30, 2024, 3:22 p.m. UTC | #5 
On So, 2024-09-29 at 20:48 +0200, Krzysztof Kozlowski wrote:
> On 29/09/2024 00:27, Al Viro wrote:
> > On Fri, Sep 27, 2024 at 04:02:32PM +0200, Philipp Zabel wrote:
> > > Use guard(mutex) to automatically unlock mutexes when going out of
> > > scope. Simplify error paths by removing a goto and manual mutex
> > > unlocking in multiple places.
> > 
> > And that, folks, is a live example of the reasons why guard() is an
> > attractive nuisance.  We really need a very loud warning on
> > cleanup.h stuff - otherwise such patches from well-meaning folks
> > will keep coming.

Thank you for the analysis. It think I'll drop this entirely.

[...]
> > 
> > Guess what happens if you take goto out_put prior to the entire thing,
> > in
> >                 ret = __reset_add_reset_gpio_device(&args);
> > 		if (ret) {
> > 			rstc = ERR_PTR(ret);
> > 			goto out_put;
> > 		}
> > That patch adds implicit mutex_unlock() at the points where we leave
> > the scope.  Which extends to the end of function.  In other words, there is
> > one downstream of out_put, turning any goto out_put upstream of guard() into
> > a bug.
> > 
> 
> cleanup.h also mentions that one should do not mix cleanup with existing
> goto, because of possibility of above issue.

Yes, d5934e76316e ("cleanup: Add usage and style documentation"), last
paragraph.

> But except careful review, this patch should have been simply compile
> tested which would point to the issue above. Any guard/scope works must
> be checked with clang W=1, which reports jumps over init.

Thank you, I was missing a CC=clang W=1 build in my pre-flight checks.
That's fixed now.

regards
Philipp
Philipp Zabel Sept. 30, 2024, 3:26 p.m. UTC | #6 
On So, 2024-09-29 at 12:45 +0200, Markus Elfring wrote:
> > Use guard(mutex) to automatically unlock mutexes when going out of
> > scope. Simplify error paths by removing a goto and manual mutex
> > unlocking in multiple places.
> …
> > +++ b/drivers/reset/core.c
> …
> @@ -1041,29 +1036,27 @@ __of_reset_control_get(struct device_node
> *node, const char *id, int index,
>  		}
>  	}
> 
> -	mutex_lock(&reset_list_mutex);
> +	guard(mutex)(&reset_list_mutex);
>  	rcdev = __reset_find_rcdev(&args, gpio_fallback);
> …
>  	rstc = __reset_control_get_internal(rcdev, rstc_id, shared,
> acquired);
> 
> -out_unlock:
> -	mutex_unlock(&reset_list_mutex);
>  out_put:
>  	of_node_put(args.np);
> …
> 
> Would you like to preserve the same lock scope (which ended before
> this function call)?

Thank you for pointing this out. Yes, and this should have alerted me
to the issue with goto out_put from before the locked region.

> @@ -1098,7 +1091,7 @@ __reset_control_get_from_lookup(struct device
> *dev, const char *con_id,
>  	const char *dev_id = dev_name(dev);
>  	struct reset_control *rstc = NULL;
> 
> -	mutex_lock(&reset_lookup_mutex);
> +	guard(mutex)(&reset_lookup_mutex);
> 
>  	list_for_each_entry(lookup, &reset_lookup_list, list) {
> …
>  			break;
>  		}
>  	}
> 
> -	mutex_unlock(&reset_lookup_mutex);
> -
>  	if (!rstc)
>  		return optional ? NULL : ERR_PTR(-ENOENT);
> …
> 
> Would you really like to increase the lock scope here?

I don't think this would have been a problem.

regards
Philipp
diff mbox series

Patch

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 4d509d41456a..6fbc6f3c14c9 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -676,25 +676,20 @@  int reset_control_acquire(struct reset_control *rstc)
 	if (reset_control_is_array(rstc))
 		return reset_control_array_acquire(rstc_to_array(rstc));
 
-	mutex_lock(&reset_list_mutex);
+	guard(mutex)(&reset_list_mutex);
 
-	if (rstc->acquired) {
-		mutex_unlock(&reset_list_mutex);
+	if (rstc->acquired)
 		return 0;
-	}
 
 	list_for_each_entry(rc, &rstc->rcdev->reset_control_head, list) {
 		if (rstc != rc && rstc->id == rc->id) {
-			if (rc->acquired) {
-				mutex_unlock(&reset_list_mutex);
+			if (rc->acquired)
 				return -EBUSY;
-			}
 		}
 	}
 
 	rstc->acquired = true;
 
-	mutex_unlock(&reset_list_mutex);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(reset_control_acquire);
@@ -1041,29 +1036,27 @@  __of_reset_control_get(struct device_node *node, const char *id, int index,
 		}
 	}
 
-	mutex_lock(&reset_list_mutex);
+	guard(mutex)(&reset_list_mutex);
 	rcdev = __reset_find_rcdev(&args, gpio_fallback);
 	if (!rcdev) {
 		rstc = ERR_PTR(-EPROBE_DEFER);
-		goto out_unlock;
+		goto out_put;
 	}
 
 	if (WARN_ON(args.args_count != rcdev->of_reset_n_cells)) {
 		rstc = ERR_PTR(-EINVAL);
-		goto out_unlock;
+		goto out_put;
 	}
 
 	rstc_id = rcdev->of_xlate(rcdev, &args);
 	if (rstc_id < 0) {
 		rstc = ERR_PTR(rstc_id);
-		goto out_unlock;
+		goto out_put;
 	}
 
 	/* reset_list_mutex also protects the rcdev's reset_control list */
 	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
 
-out_unlock:
-	mutex_unlock(&reset_list_mutex);
 out_put:
 	of_node_put(args.np);
 
@@ -1098,7 +1091,7 @@  __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 	const char *dev_id = dev_name(dev);
 	struct reset_control *rstc = NULL;
 
-	mutex_lock(&reset_lookup_mutex);
+	guard(mutex)(&reset_lookup_mutex);
 
 	list_for_each_entry(lookup, &reset_lookup_list, list) {
 		if (strcmp(lookup->dev_id, dev_id))
@@ -1107,11 +1100,9 @@  __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 		if ((!con_id && !lookup->con_id) ||
 		    ((con_id && lookup->con_id) &&
 		     !strcmp(con_id, lookup->con_id))) {
-			mutex_lock(&reset_list_mutex);
+			guard(mutex)(&reset_list_mutex);
 			rcdev = __reset_controller_by_name(lookup->provider);
 			if (!rcdev) {
-				mutex_unlock(&reset_list_mutex);
-				mutex_unlock(&reset_lookup_mutex);
 				/* Reset provider may not be ready yet. */
 				return ERR_PTR(-EPROBE_DEFER);
 			}
@@ -1119,13 +1110,10 @@  __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 			rstc = __reset_control_get_internal(rcdev,
 							    lookup->index,
 							    shared, acquired);
-			mutex_unlock(&reset_list_mutex);
 			break;
 		}
 	}
 
-	mutex_unlock(&reset_lookup_mutex);
-
 	if (!rstc)
 		return optional ? NULL : ERR_PTR(-ENOENT);