Re: [PATCH] reset: Further simplify locking with guard()
From: Philipp Zabel <p.zabel@pengutronix.de>
Date: 2024-09-30 15:27:02
Also in:
kernel-janitors, lkml
On So, 2024-09-29 at 12:45 +0200, Markus Elfring wrote:
quoted hunk ↗ jump to hunk
quoted
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.…quoted
+++ 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.
quoted hunk ↗ jump to hunk
@@ -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