On Sun, Feb 17, 2019 at 06:55:39PM +0100, Andrew Lunn wrote:
> On Sun, Feb 17, 2019 at 04:27:32PM +0000, Russell King wrote:
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 4.20.0+ #302 Not tainted
> > ------------------------------------------------------
> > systemd-udevd/160 is trying to acquire lock:
> > edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
> >
> > but task is already holding lock:
> > edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
> >
> > which lock already depends on the new lock.
> >
> > the existing dependency chain (in reverse order) is:
> >
> > -> #1 (&desc->request_mutex){+.+.}:
> > mutex_lock_nested+0x1c/0x24
> > __setup_irq+0xa0/0x704
> > request_threaded_irq+0xd0/0x150
> > mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]
>
> > -> #0 (&chip->reg_lock){+.+.}:
> > __mutex_lock+0x50/0x8b8
> > mutex_lock_nested+0x1c/0x24
> > __setup_irq+0x640/0x704
> > request_threaded_irq+0xd0/0x150
> > mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
> > mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
> > mdio_probe+0x2c/0x54
> >
> > other info that might help us debug this:
> >
> > Possible unsafe locking scenario:
> >
> > CPU0 CPU1
> > ---- ----
> > lock(&desc->request_mutex);
> > lock(&chip->reg_lock);
> > lock(&desc->request_mutex);
> > lock(&chip->reg_lock);
>
> Hi Russell
>
> I failed to reproduce it on a Freescale system. Which made me take a
> closer look at the above. This is a false positive.
>
> In #1 we are requesting the GPIO interrupt. In #2 we are requesting
> the chained interrupt from the mv88e6xxx global 1 interrupt handler.
> So these are different desc->request_mutex. The Freescale VF610 GPIO
> driver uses gpiochip_irqchip_add(), which creates a lock class for the
> GPIO. The marvell gpio-mvebu driver does not create a lock class. So
> when i test on Freescale, lockdep can tell they are different mutex,
> but on clearfog it cannot.
>
> So i think the real fix is probably two fold, although just doing one
> is sufficient:
>
> 1) Add lock classes to gpio-mvebu, by call irq_set_lockdep_class()
> 2) Add lock classes to chip.c global 1, by calling irq_set_lockdep_class()
>
> There is probably more value in 1) since the mvebu gpio driver is much
> more widely used than the mv88e6xxx driver.
I'd ask one question: is there any reason to hold chip->reg_lock while
calling request_threaded_irq()? If not, then surely it would be best
not to do so?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
