call for bge(4) testers
Oleg Bulyzhin
oleg at freebsd.org
Thu Aug 24 19:26:24 UTC 2006
On Thu, Aug 24, 2006 at 10:41:21AM +0900, Pyun YongHyeon wrote:
> On Thu, Aug 24, 2006 at 05:24:50AM +0400, Oleg Bulyzhin wrote:
> > On Thu, Aug 24, 2006 at 10:11:16AM +0900, Pyun YongHyeon wrote:
> > > On Thu, Aug 24, 2006 at 04:42:25AM +0400, Oleg Bulyzhin wrote:
> > > > On Thu, Aug 24, 2006 at 09:26:32AM +0900, Pyun YongHyeon wrote:
> > > > > On Wed, Aug 23, 2006 at 04:40:35PM +0400, Oleg Bulyzhin wrote:
> > >
> > > [...]
> > >
> > > > >
> > > > > > My idea was: perhaps, under certain condition, concurrent access to PHY could
> > > > > > lead to hardware deadlock.
> > > > > >
> > > > >
> > > > > Yes. Because MII bus access needs several steps to access PHY
> > > > > registers its operation shouldn't be interrupted until all pending
> > > > > requests are served.
> > > > >
> > > > > I can't sure you remember my mail for MII lock which modifies
> > > > > mii_phy_probe API to take an additional mutex. The driver mutex
> > > > > could be used with MII bus access/callbacks.
> > > >
> > > > Yes, i remember that mail but i didnt had time to dig code deep enough
> > > > to give an answer. I did it recently, while working on PR.
> > > >
> > >
> > > Glad to hear that. :-)
> > >
> > > > > If interface is up/running and auto negotiation is in progress MII
> > > > > layer would inspect BMSR register periodically to know the state
> > > > > of link. During the time if you run ifconfig(8) to know the state
> > > > > of the link or to change media type/duplex it will access PHY
> > > > > registers. Normally it would end up with "link states coalesced"
> > > > > messages.
> > > > >
> > > > > As you know the two callbacks(vge_ifmedia_upd/vge_ifmedia_sts) will
> > > > > end up with calling mii_mediachg() or mii_pollstat() which in turn
> > > > > access PHY registers. So if MII access is properly serialized we
> > > > > wouldn't get stale data. I guess your fix solves it by protecting
> > > > > callbacks with driver mutex but it wouldn't fix other cases.
> > > > > For example see vge_miibus_statchg MII interface.
> > > >
> > > > MII layer does not have it's own callouts, i.e. those autonegotiation
> > > > checks of BMSR are triggered by driver's _tick() function, which are locked.
> > > > So if we have locked _tick() & _ifmedia_(upd|sts) functions, concurrent
> > > > access to PHY is impossible.
> > > > (If we are talking about vge_miibus_statchg(), it would be:
> > > > vge_tick (here we obtain lock) -> mii_tick -> ciphy_service ->
> > > > mii_phy_update -> vge_miibus_statchg).
> > > >
> > >
> > > If we set media type manually while autonegotiation is in progress,
> > > wouldn't it access PHY registers without driver lock?
> >
> > No (if we have _tick() & ifmedia callbacks locked):
> > ioctl -> netioctl -> ifhwioctl -> vge_ioctl(SIOCSIFMEDIA) -> ifmedia_ioctl ->
> > vge_ifmedia_upd (here we obtain lock) -> mii_mediachg ->
> > ciphy_service(MII_MEDIACHG) -> mii_phy_update -> vge_miibus_statchg
> >
>
> Ah...I've missed the your assumption ifmedia callbacks were locked.
> As you said, several drivers don't have locks for ifmedia callback
> handlers. These drivers would access PHY registers without locks.
I believe all such drivers should be fixed.
>
> --
> Regards,
> Pyun YongHyeon
--
Oleg.
More information about the freebsd-current
mailing list