Re: [PATCH v3] devres: Refactor using guards

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux