Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 24 Aug 2006 23:26:21 +0400
From:      Oleg Bulyzhin <oleg@freebsd.org>
To:        Pyun YongHyeon <pyunyh@gmail.com>
Cc:        Michael Reifenberger <mike@Reifenberger.com>, freebsd-current@freebsd.org
Subject:   Re: call for bge(4) testers
Message-ID:  <20060824192621.GB39797@lath.rinet.ru>
In-Reply-To: <20060824014121.GE22634@cdnetworks.co.kr>
References:  <20060822073201.GI12848@cdnetworks.co.kr> <20060822144341.L5561@fw.reifenberger.com> <20060822204342.GA4943@lath.rinet.ru> <20060823005554.GC17902@cdnetworks.co.kr> <20060823124035.GA18628@lath.rinet.ru> <20060824002632.GA22634@cdnetworks.co.kr> <20060824004225.GB25876@lath.rinet.ru> <20060824011116.GD22634@cdnetworks.co.kr> <20060824012450.GC27699@lath.rinet.ru> <20060824014121.GE22634@cdnetworks.co.kr>

next in thread | previous in thread | raw e-mail | index | archive | help
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.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060824192621.GB39797>