Messages in this thread | | | Date | Fri, 14 Sep 2018 15:29:59 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net-next 1/5] net: phy: mscc: add ethtool statistics counters |
| |
> When you change a page, you basically can access only the registers in > this page so if there are two functions requesting different pages at > the same time or registers of different pages, it won't work well > indeed. > > > phy_read_page() and phy_write_page() will do the needed locking if > > this is an issue. > > > > That's awesome! Didn't know it existed. Thanks a ton! > > Well, that means I should migrate the whole driver to use > phy_read/write_paged instead of the phy_read/write that is currently in > use. > > That's impacting performance though as per phy_read/write_paged we read > the current page, set the desired page, read/write the register, set the > old page back. That's 4 times more operations.
You can use the lower level locking primatives. See m88e1318_set_wol() for example.
> Couldn't we use the > phy_device mutex instead (as it's currently done in the whole driver)? > Or is it worse/comparable in performance to the suggested solution?
Russell King found a race condition where this breaks. You cannot hold the phy_device mutex everywhere.
Andrew
| |