Hi Greg,
On 12/06/2024 12:05, Greg KH wrote:
On Wed, Jun 12, 2024 at 12:00:06PM +0200, Andrea Calabrese wrote:
Hi Lucas,
On 12/06/2024 11:26, Lucas Stach wrote:
Am Dienstag, dem 11.06.2024 um 11:37 +0200 schrieb Andrea Calabrese:
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@xxxxxxxxxxxxxxxxxxxx>
---
Changed commit message from V2. Also changed title, shortened the file
name.
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 3df0025d12aa..a98720e0cb2b 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)(&dev->devres_lock);
This is not equivalent to the current code. You are dropping the
_irqsave part of the locking scheme, totally changing the semantics
here. This issue is present in multiple places in this patch.
Regards,
Lucas
I don't see where is the issue here, as I am using both guard and
scoped_guard similarly to how they are used in drivers/gpio/gpiolib-cdev.c,
which I took as a reference for the usage of the construct. If so, probably
we may both be using it wrong.
Look at the difference between calling:
spin_lock(&lock);
and
spin_lock_irqsave(&lock, flags);
They are functionally NOT the same.
thanks,
greg k-h
Ouch, I saw it now... I will fix it and send a V4.
Thank you!
Andrea
