lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 11 Mar 2023 16:19:43 +0100 From: Andrew Lunn <andrew@...n.ch> To: Klaus Kudielka <klaus.kudielka@...il.com> Cc: Florian Fainelli <f.fainelli@...il.com>, Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 2/2] net: dsa: mv88e6xxx: move call to mv88e6xxx_mdios_register() On Sat, Mar 11, 2023 at 10:41:41AM +0100, Klaus Kudielka wrote: > >From commit 1a136ca2e089 ("net: mdio: scan bus based on bus capabilities > for C22 and C45") onwards, mdiobus_scan_bus_c45() is being called on buses > with MDIOBUS_NO_CAP. On a Turris Omnia (Armada 385, 88E6176 switch), this > causes a significant increase of boot time, from 1.6 seconds, to 6.3 > seconds. The boot time stated here is until start of /init. > > Further testing revealed that the C45 scan is indeed expensive (around > 2.7 seconds, due to a huge number of bus transactions), and called twice. > > It was suggested, to call mv88e6xxx_mdios_register() at the beginning of > mv88e6xxx_setup(), and mv88e6xxx_mdios_unregister() at the end of > mv88e6xxx_teardown(). This is accomplished by this patch. > > Testing on the Turris Omnia revealed, that this improves the situation. > Now mdiobus_scan_bus_c45() is called only once, ending up in a boot time > of 4.3 seconds. For those who are interested, here is a bit of background on why this change reduces the number of bus scans. The MAC driver probes, which i think in this case is mvneta. Part way through its probe, it registers its MDIO bus. That triggers a scan of its bus, and the switch is found. The mv88e6xxx driver is then loaded and its probe function called. towards the end of the mv88e6xxxx probe function, it registers its MDIO bus. That causes a scan of the switches MDIO bus. Which is slow. After the scan completes, the mv88e6xxx probe continues, and registers the switch with DSA core. The core then parses the DT binding for the switch and looks for the master ethernet interface. That is the interface which mvneta provides. But mvneta is still only part way through its probe. It has not yet registered its interface with the netdev core. So the DSA core fails to find it and return EPROBE_DEFER. This causes the mv88e6xxx driver to unwind its probe. The mvneat then gets a chance to finish its probe and register its netdev. Some timer later, the driver core runs the probes again for those drivers which returned EPROBE_DEFER, mv88e6xxx registers its MDIO bus again, another scan is performed, the switch is registered with the code, and this time the master device is available, so things continue. The DSA core then calls the drivers .setup() callback to get the switch into a usable state. I think what remains in the probe function is cheap, so it can probable stay there and be done twice. But it might be worth putting in a few printks to get some time stamps and see if anything is expensive. > static int mv88e6xxx_setup(struct dsa_switch *ds) > @@ -3889,6 +3892,10 @@ static int mv88e6xxx_setup(struct dsa_switch *ds) > int err; > int i; > > + err = mv88e6xxx_mdios_register(chip); > + if (err) > + return err; > + > chip->ds = ds; > ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip); Other calls in mv88e6xxx_setup() can fail, so you need to extend the cleanup to remove the mdio bus on failure. Andrew
Powered by blists - more mailing lists