Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 17 Feb 2022 13:12:19 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Alvin =?utf-8?Q?=C5=A0ipraga?= <ALSI@bang-olufsen.dk>
Cc:     Luiz Angelo Daros de Luca <luizluca@gmail.com>,
        Alvin =?utf-8?Q?=C5=A0ipraga?= <alvin@pqrs.dk>,
        Linus Walleij <linus.walleij@linaro.org>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Michael Rasmussen <MIR@bang-olufsen.dk>,
        =?utf-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>,
        "open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 0/2] net: dsa: realtek: fix PHY register read
 corruption
Message-ID: <Yg47o5619InYrs9x@lunn.ch>
References: <20220216160500.2341255-1-alvin@pqrs.dk>
 <CAJq09z6Mr7QFSyqWuM1jjm9Dis4Pa2A4yi=NJv1w4FM0WoyqtA@mail.gmail.com>
 <87k0dusmar.fsf@bang-olufsen.dk>
 <Yg1MfpK5PwiAbGfU@lunn.ch>
 <878ruasjd8.fsf@bang-olufsen.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <878ruasjd8.fsf@bang-olufsen.dk>
Precedence: bulk

> Thank you Andrew for the clear explanation.
> 
> Somewhat unrelated to this series, but are you able to explain to me the
> difference between:
> 
> 	mutex_lock(&bus->mdio_lock);
> and
> 	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
> 
> While looking at other driver examples I noticed the latter form quite a
> few times too.

This is to do with the debug code for checking for deadlocks,
CONFIG_PROVE_LOCKING. When that feature is enables, each lock/unlock
of a mutex is tracked, and a list is made of what other locks are also
taken, and the order. The code can find deadlocks where one thread
takes A then B, while another thread takes B and then A. It can also
detect when a thread takes lock A and then tries to take lock A again.

Rather than track each individual mutex, it uses classes of mutex. So
bus->mdio_lock is a class of mutex. The code simply tracks that a
bus->mdio_lock has been taken, not a specific bus->mdio_lock. That is
generally sufficient, but not always. The mv88e6xxx switch is like
many switches, accessed over MDIO. But the mv88e6xxx switch offers an
MDIO bus, and there is an MDIO bus driver inside the mv88e6xxx
driver. So you have nested MDIO calls. So this debug code seems the
same class of mutex being taken twice, and thinks it is a
deadlock. You can tell it that nested MDIO calls are actually O.K, it
won't deadlock.

      Andrew