Re: [PATCH] USB host: Fix lockdep warning in AMD PLL quirk

[Roedel, Joerg]

On Thu, Apr 07, 2011 at 05:58:14AM -0400, Xu, Andiry wrote:
> > +     spin_lock_irqsave(&amd_lock, flags);
> > +     if (amd_chipset.probe_count > 0) {
> > +             /* race - someone else was faster - drop devices */
> > +             if (info.nb_dev)
> > +                     pci_dev_put(info.nb_dev);
> > +             if (info.smbus_dev)
> > +                     pci_dev_put(info.smbus_dev);
> 
> You need to increase info.probe_count here and set
> amd_chipset.probe_count with it, because it's decreased during every
> host controller stop routine.

Right, the correct return value needs to be set too. I'll fix it.

> > +     } else {
> > +             /* no race - commit the result */
> > +             info.probe_count++;
> > +             amd_chipset = info;
> > +     }
> >       spin_unlock_irqrestore(&amd_lock, flags);
> > -     return amd_chipset.probe_result;
> > +
> > +     return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(usb_amd_find_chipset_info);
> >
> > --
> > 1.7.1
> 
> The original design target is to probe the chipset only once, so other
> host controllers will not probe the chipset again. This patch allows
> multiple threads to probe the chipset at the same time, and only one
> thread is doing the valid work. It may add unnecessary cost.

Not necessarily. With this patch more than one thread could do the valid
work at the same time. With the old code the other threads would
have spent this time spinning on the lock. So I don't think there is
much additional cost introduced with this patch. And as a plus, it fixes
a bug :-)
I'll send a fixed version soon.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/