[RESEND PATCH v4] devres: Refactor using guards
Andrea Calabrese
andrea.calabrese at amarulasolutions.com
Tue Sep 10 13:15:21 UTC 2024
Code refactoring using the recent guard and scoped_guard macros
for automatic cleanup of the spinlocks. This does not change the
effective behaviour of the kernel, but guarantees a cleaned-up exit from
each lock, automatically avoiding potential deadlocks.
Signed-off-by: Andrea Calabrese <andrea.calabrese at amarulasolutions.com>
---
Diff from V3: as Greg KH and Lucas Stach noticed, there was a
behavioural change between the two versions: I used guard(spinlock),
while the expected behaviour should have come from a spinlock_irqsave
guard. This has been fixed.
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 3df0025d12aa..4872756426dd 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -194,14 +194,12 @@ void devres_for_each_res(struct device *dev, dr_release_t release,
{
struct devres_node *node;
struct devres_node *tmp;
- unsigned long flags;
if (!fn)
return;
- spin_lock_irqsave(&dev->devres_lock, flags);
- list_for_each_entry_safe_reverse(node, tmp,
- &dev->devres_head, entry) {
+ guard(spinlock_irqsave)(&dev->devres_lock);
+ list_for_each_entry_safe_reverse(node, tmp, &dev->devres_head, entry) {
struct devres *dr = container_of(node, struct devres, node);
if (node->release != release)
@@ -210,7 +208,6 @@ void devres_for_each_res(struct device *dev, dr_release_t release,
continue;
fn(dev, dr->data, data);
}
- spin_unlock_irqrestore(&dev->devres_lock, flags);
}
EXPORT_SYMBOL_GPL(devres_for_each_res);
@@ -243,11 +240,9 @@ EXPORT_SYMBOL_GPL(devres_free);
void devres_add(struct device *dev, void *res)
{
struct devres *dr = container_of(res, struct devres, data);
- unsigned long flags;
- spin_lock_irqsave(&dev->devres_lock, flags);
+ guard(spinlock_irqsave)(&dev->devres_lock);
add_dr(dev, &dr->node);
- spin_unlock_irqrestore(&dev->devres_lock, flags);
}
EXPORT_SYMBOL_GPL(devres_add);
@@ -287,11 +282,8 @@ void * devres_find(struct device *dev, dr_release_t release,
dr_match_t match, void *match_data)
{
struct devres *dr;
- unsigned long flags;
-
- spin_lock_irqsave(&dev->devres_lock, flags);
+ guard(spinlock_irqsave)(&dev->devres_lock);
dr = find_dr(dev, release, match, match_data);
- spin_unlock_irqrestore(&dev->devres_lock, flags);
if (dr)
return dr->data;
@@ -318,16 +310,14 @@ void * devres_get(struct device *dev, void *new_res,
{
struct devres *new_dr = container_of(new_res, struct devres, data);
struct devres *dr;
- unsigned long flags;
-
- spin_lock_irqsave(&dev->devres_lock, flags);
- dr = find_dr(dev, new_dr->node.release, match, match_data);
- if (!dr) {
- add_dr(dev, &new_dr->node);
- dr = new_dr;
- new_res = NULL;
+ scoped_guard(spinlock_irqsave, &dev->devres_lock) {
+ dr = find_dr(dev, new_dr->node.release, match, match_data);
+ if (!dr) {
+ add_dr(dev, &new_dr->node);
+ dr = new_dr;
+ new_res = NULL;
+ }
}
- spin_unlock_irqrestore(&dev->devres_lock, flags);
devres_free(new_res);
return dr->data;
@@ -353,15 +343,13 @@ void * devres_remove(struct device *dev, dr_release_t release,
dr_match_t match, void *match_data)
{
struct devres *dr;
- unsigned long flags;
-
- spin_lock_irqsave(&dev->devres_lock, flags);
- dr = find_dr(dev, release, match, match_data);
- if (dr) {
- list_del_init(&dr->node.entry);
- devres_log(dev, &dr->node, "REM");
+ scoped_guard(spinlock_irqsave, &dev->devres_lock) {
+ dr = find_dr(dev, release, match, match_data);
+ if (dr) {
+ list_del_init(&dr->node.entry);
+ devres_log(dev, &dr->node, "REM");
+ }
}
- spin_unlock_irqrestore(&dev->devres_lock, flags);
if (dr)
return dr->data;
@@ -516,7 +504,6 @@ static void release_nodes(struct device *dev, struct list_head *todo)
*/
int devres_release_all(struct device *dev)
{
- unsigned long flags;
LIST_HEAD(todo);
int cnt;
@@ -528,9 +515,9 @@ int devres_release_all(struct device *dev)
if (list_empty(&dev->devres_head))
return 0;
- spin_lock_irqsave(&dev->devres_lock, flags);
- cnt = remove_nodes(dev, dev->devres_head.next, &dev->devres_head, &todo);
- spin_unlock_irqrestore(&dev->devres_lock, flags);
+ scoped_guard(spinlock_irqsave, &dev->devres_lock) {
+ cnt = remove_nodes(dev, dev->devres_head.next, &dev->devres_head, &todo);
+ }
release_nodes(dev, &todo);
return cnt;
@@ -552,7 +539,6 @@ int devres_release_all(struct device *dev)
void * devres_open_group(struct device *dev, void *id, gfp_t gfp)
{
struct devres_group *grp;
- unsigned long flags;
grp = kmalloc(sizeof(*grp), gfp);
if (unlikely(!grp))
@@ -568,9 +554,8 @@ void * devres_open_group(struct device *dev, void *id, gfp_t gfp)
if (id)
grp->id = id;
- spin_lock_irqsave(&dev->devres_lock, flags);
+ guard(spinlock_irqsave)(&dev->devres_lock);
add_dr(dev, &grp->node[0]);
- spin_unlock_irqrestore(&dev->devres_lock, flags);
return grp->id;
}
EXPORT_SYMBOL_GPL(devres_open_group);
@@ -609,17 +594,14 @@ static struct devres_group * find_group(struct device *dev, void *id)
void devres_close_group(struct device *dev, void *id)
{
struct devres_group *grp;
- unsigned long flags;
- spin_lock_irqsave(&dev->devres_lock, flags);
+ guard(spinlock_irqsave)(&dev->devres_lock);
grp = find_group(dev, id);
if (grp)
add_dr(dev, &grp->node[1]);
else
WARN_ON(1);
-
- spin_unlock_irqrestore(&dev->devres_lock, flags);
}
EXPORT_SYMBOL_GPL(devres_close_group);
@@ -635,19 +617,16 @@ EXPORT_SYMBOL_GPL(devres_close_group);
void devres_remove_group(struct device *dev, void *id)
{
struct devres_group *grp;
- unsigned long flags;
-
- spin_lock_irqsave(&dev->devres_lock, flags);
-
- grp = find_group(dev, id);
- if (grp) {
- list_del_init(&grp->node[0].entry);
- list_del_init(&grp->node[1].entry);
- devres_log(dev, &grp->node[0], "REM");
- } else
- WARN_ON(1);
- spin_unlock_irqrestore(&dev->devres_lock, flags);
+ scoped_guard(spinlock_irqsave, &dev->devres_lock) {
+ grp = find_group(dev, id);
+ if (grp) {
+ list_del_init(&grp->node[0].entry);
+ list_del_init(&grp->node[1].entry);
+ devres_log(dev, &grp->node[0], "REM");
+ } else
+ WARN_ON(1);
+ }
kfree(grp);
}
@@ -668,11 +647,10 @@ EXPORT_SYMBOL_GPL(devres_remove_group);
int devres_release_group(struct device *dev, void *id)
{
struct devres_group *grp;
- unsigned long flags;
LIST_HEAD(todo);
int cnt = 0;
- spin_lock_irqsave(&dev->devres_lock, flags);
+ guard(spinlock_irqsave)(&dev->devres_lock);
grp = find_group(dev, id);
if (grp) {
@@ -683,12 +661,9 @@ int devres_release_group(struct device *dev, void *id)
end = grp->node[1].entry.next;
cnt = remove_nodes(dev, first, end, &todo);
- spin_unlock_irqrestore(&dev->devres_lock, flags);
-
release_nodes(dev, &todo);
} else {
WARN_ON(1);
- spin_unlock_irqrestore(&dev->devres_lock, flags);
}
return cnt;
@@ -860,7 +835,6 @@ void *devm_krealloc(struct device *dev, void *ptr, size_t new_size, gfp_t gfp)
{
size_t total_new_size, total_old_size;
struct devres *old_dr, *new_dr;
- unsigned long flags;
if (unlikely(!new_size)) {
devm_kfree(dev, ptr);
@@ -906,20 +880,17 @@ void *devm_krealloc(struct device *dev, void *ptr, size_t new_size, gfp_t gfp)
* The spinlock protects the linked list against concurrent
* modifications but not the resource itself.
*/
- spin_lock_irqsave(&dev->devres_lock, flags);
+ scoped_guard(spinlock_irqsave, &dev->devres_lock) {
+ old_dr = find_dr(dev, devm_kmalloc_release, devm_kmalloc_match, ptr);
+ if (!old_dr) {
+ kfree(new_dr);
+ WARN(1, "Memory chunk not managed or managed by a different device.");
+ return NULL;
+ }
- old_dr = find_dr(dev, devm_kmalloc_release, devm_kmalloc_match, ptr);
- if (!old_dr) {
- spin_unlock_irqrestore(&dev->devres_lock, flags);
- kfree(new_dr);
- WARN(1, "Memory chunk not managed or managed by a different device.");
- return NULL;
+ replace_dr(dev, &old_dr->node, &new_dr->node);
}
- replace_dr(dev, &old_dr->node, &new_dr->node);
-
- spin_unlock_irqrestore(&dev->devres_lock, flags);
-
/*
* We can copy the memory contents after releasing the lock as we're
* no longer modifying the list links.
--
2.34.1
More information about the dri-devel
mailing list