* [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat
@ 2015-10-21 15:37 Neil Armstrong
2015-10-21 16:14 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Neil Armstrong @ 2015-10-21 15:37 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, Guenter Roeck, vivien.didelot,
Andrew Lunn, Fabian Frederick, Pavel Nakonechny, Joe Perches,
netdev, linux-kernel, nbd, sergei.shtylyov
Like the change made for mv88e6xxx, use mutex_lock_nested() to avoid
lockdep to give false positives because of nested MDIO busses.
The false positive was observed using a mv88e6060 from a TI816X SoC.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/net/dsa/mv88e6060.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index c29aebe..b1db460 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -19,14 +19,24 @@
#define REG_PORT(p) (8 + (p))
#define REG_GLOBAL 0x0f
+/* MDIO bus access can be nested in the case of PHYs connected to the
+ * internal MDIO bus of the switch, which is accessed via MDIO bus of
+ * the Ethernet interface. Avoid lockdep false positives by using
+ * mutex_lock_nested().
+ */
static int reg_read(struct dsa_switch *ds, int addr, int reg)
{
+ int ret;
struct mii_bus *bus = dsa_host_dev_to_mii_bus(ds->master_dev);
if (bus == NULL)
return -EINVAL;
- return mdiobus_read(bus, ds->pd->sw_addr + addr, reg);
+ mutex_lock_nested(&bus->mdio_lock, SINGLE_DEPTH_NESTING);
+ ret = bus->read(bus, ds->pd->sw_addr, reg);
+ mutex_unlock(&bus->mdio_lock);
+
+ return ret;
}
#define REG_READ(addr, reg) \
@@ -42,12 +52,17 @@ static int reg_read(struct dsa_switch *ds, int addr, int reg)
static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
{
+ int ret;
struct mii_bus *bus = dsa_host_dev_to_mii_bus(ds->master_dev);
if (bus == NULL)
return -EINVAL;
- return mdiobus_write(bus, ds->pd->sw_addr + addr, reg, val);
+ mutex_lock_nested(&bus->mdio_lock, SINGLE_DEPTH_NESTING);
+ ret = bus->write(bus, ds->pd->sw_addr, reg, val);
+ mutex_unlock(&bus->mdio_lock);
+
+ return ret;
}
#define REG_WRITE(addr, reg, val) \
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat
2015-10-21 15:37 [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat Neil Armstrong
@ 2015-10-21 16:14 ` Andrew Lunn
2015-10-22 7:33 ` Neil Armstrong
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andrew Lunn @ 2015-10-21 16:14 UTC (permalink / raw)
To: Neil Armstrong
Cc: David S. Miller, Florian Fainelli, Guenter Roeck, vivien.didelot,
Fabian Frederick, Pavel Nakonechny, Joe Perches, netdev,
linux-kernel, nbd, sergei.shtylyov
On Wed, Oct 21, 2015 at 05:37:45PM +0200, Neil Armstrong wrote:
> Like the change made for mv88e6xxx, use mutex_lock_nested() to avoid
> lockdep to give false positives because of nested MDIO busses.
Hi Neil
We now have three instances of this, since mdio-mux.c has the same
code. Maybe now would be a good time to refactor this code into
mdiobus_read_nested() and mdiobus_write_nested() in mdio_bus.c? At
the same time, add BUG_ON(in_interrupt()) similar to the non-nested
versions?
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat
2015-10-21 16:14 ` Andrew Lunn
@ 2015-10-22 7:33 ` Neil Armstrong
2015-10-22 8:08 ` Neil Armstrong
2015-10-22 8:08 ` Neil Armstrong
2 siblings, 0 replies; 6+ messages in thread
From: Neil Armstrong @ 2015-10-22 7:33 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Florian Fainelli, Guenter Roeck, vivien.didelot,
Fabian Frederick, Pavel Nakonechny, Joe Perches, netdev,
linux-kernel, nbd, sergei.shtylyov
Hi Andrew,
On 10/21/2015 06:14 PM, Andrew Lunn wrote:
> On Wed, Oct 21, 2015 at 05:37:45PM +0200, Neil Armstrong wrote:
>> Like the change made for mv88e6xxx, use mutex_lock_nested() to avoid
>> lockdep to give false positives because of nested MDIO busses.
>
> Hi Neil
>
> We now have three instances of this, since mdio-mux.c has the same
> code. Maybe now would be a good time to refactor this code into
> mdiobus_read_nested() and mdiobus_write_nested() in mdio_bus.c? At
> the same time, add BUG_ON(in_interrupt()) similar to the non-nested
> versions?
>
> Andrew
>
Indeed, you are right, I will post a serie with this refactoring.
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat
2015-10-21 16:14 ` Andrew Lunn
2015-10-22 7:33 ` Neil Armstrong
@ 2015-10-22 8:08 ` Neil Armstrong
2015-10-22 8:08 ` Neil Armstrong
2 siblings, 0 replies; 6+ messages in thread
From: Neil Armstrong @ 2015-10-22 8:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Florian Fainelli, Guenter Roeck, vivien.didelot,
Fabian Frederick, Pavel Nakonechny, Joe Perches, netdev,
linux-kernel, nbd, sergei.shtylyov
On 10/21/2015 06:14 PM, Andrew Lunn wrote:
> On Wed, Oct 21, 2015 at 05:37:45PM +0200, Neil Armstrong wrote:
>> Like the change made for mv88e6xxx, use mutex_lock_nested() to avoid
>> lockdep to give false positives because of nested MDIO busses.
>
> Hi Neil
>
> We now have three instances of this, since mdio-mux.c has the same
> code. Maybe now would be a good time to refactor this code into
> mdiobus_read_nested() and mdiobus_write_nested() in mdio_bus.c? At
> the same time, add BUG_ON(in_interrupt()) similar to the non-nested
> versions?
>
> Andrew
>
Well, mdio-mux also calls switch_fn inside the mdio_lock, clean refactoring
would introduce a separate lock and call the nested variants.
Is that ok ? Can someone test mdio-mux is I make the change ?
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat
2015-10-21 16:14 ` Andrew Lunn
2015-10-22 7:33 ` Neil Armstrong
2015-10-22 8:08 ` Neil Armstrong
@ 2015-10-22 8:08 ` Neil Armstrong
2015-10-22 12:06 ` Andrew Lunn
2 siblings, 1 reply; 6+ messages in thread
From: Neil Armstrong @ 2015-10-22 8:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Florian Fainelli, Guenter Roeck, vivien.didelot,
Fabian Frederick, Pavel Nakonechny, Joe Perches, netdev,
linux-kernel, nbd, sergei.shtylyov
On 10/21/2015 06:14 PM, Andrew Lunn wrote:
> On Wed, Oct 21, 2015 at 05:37:45PM +0200, Neil Armstrong wrote:
>> Like the change made for mv88e6xxx, use mutex_lock_nested() to avoid
>> lockdep to give false positives because of nested MDIO busses.
>
> Hi Neil
>
> We now have three instances of this, since mdio-mux.c has the same
> code. Maybe now would be a good time to refactor this code into
> mdiobus_read_nested() and mdiobus_write_nested() in mdio_bus.c? At
> the same time, add BUG_ON(in_interrupt()) similar to the non-nested
> versions?
>
> Andrew
>
Well, mdio-mux also calls switch_fn inside the mdio_lock, clean refactoring
would introduce a separate lock and call the nested variants.
Is that ok ? Can someone test mdio-mux if I make the change ?
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat
2015-10-22 8:08 ` Neil Armstrong
@ 2015-10-22 12:06 ` Andrew Lunn
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2015-10-22 12:06 UTC (permalink / raw)
To: Neil Armstrong
Cc: David S. Miller, Florian Fainelli, Guenter Roeck, vivien.didelot,
Fabian Frederick, Pavel Nakonechny, Joe Perches, netdev,
linux-kernel, nbd, sergei.shtylyov
> Well, mdio-mux also calls switch_fn inside the mdio_lock, clean refactoring
> would introduce a separate lock and call the nested variants.
> Is that ok ? Can someone test mdio-mux if I make the change ?
Hi Neil
I would not touch mdio-mux. As you said, it does more than lock, read,
unlock. It is not something sufficiently generic to place into shared
code.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-22 12:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-21 15:37 [PATCH] net: dsa: mv88e6060: Fix false positive lockdep splat Neil Armstrong
2015-10-21 16:14 ` Andrew Lunn
2015-10-22 7:33 ` Neil Armstrong
2015-10-22 8:08 ` Neil Armstrong
2015-10-22 8:08 ` Neil Armstrong
2015-10-22 12:06 ` Andrew Lunn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).