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
