Date: Thu, 24 Aug 2006 04:42:25 +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: <20060824004225.GB25876@lath.rinet.ru> In-Reply-To: <20060824002632.GA22634@cdnetworks.co.kr> References: <20060822042023.GC12848@cdnetworks.co.kr> <20060822091107.A3909@fw.reifenberger.com> <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>
next in thread | previous in thread | raw e-mail | index | archive | help
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: > > On Wed, Aug 23, 2006 at 09:55:54AM +0900, Pyun YongHyeon wrote: > > > On Wed, Aug 23, 2006 at 12:43:42AM +0400, Oleg Bulyzhin wrote: > > > > On Tue, Aug 22, 2006 at 02:44:34PM +0200, Michael Reifenberger wrote: > > > > > On Tue, 22 Aug 2006, Pyun YongHyeon wrote: > > > > > ... > > > > > >I'm not familiar with vge(4) and don't have hardwares supported by > > > > > >vge(4). Because vge(4) supports a kind of interrupt moderation, there > > > > > >is a possiblity to have the same issue seen on em(4). > > > > > >If you want my blind patch I can send a patch for you. > > > > > > > > > > > Yes, please! > > > > > I can test it (on RELENG_6 though). > > > > > > > > I have an idea why those timeouts can happen. Could you please test > > > > attached patch? It may help (or may not). Anyway would be fine > > > > to know results. > > > > > > > > > > Since vge(4) uses MTX_RECURSE mutex and miibus(4) handler is > > > protected with the mutex I guess it wouldn't help much. > > > I guess it needs a seperate mutex to protect miibus(4) handler > > > and should remove the use of MTX_RECURSE. > > > > Hmm. > > 1) _ifmedia_upd() & _ifmedia_sts() functions are not called from mii layer. > > 2) As i can see MII layer is not protected by anything, unless you > > specially acquire driver lock prior to calling mii_ function. > > Locking ifmedia callbacks should be done (though, it may not help > > with watchdogs timeout), otherwise we have race on accessing PHY registers. > > (kern/98738). > > > > As i can see, random watchdog timeouts was reported for em, bge, vge, sk > > (and maybe others, those ones which i remember) drivers. > > All of them has unlocked _ifmedia_ functions. > > > > AFAIK all known sk(4) bug were fixed. If it's not please let me know. > > > 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. > 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). > > > > > > vge(4) also has a bug > > > if mbuf chain is too long(7 or higher) and defragmentation with > > > m_defrag(9) fails it would access an invalid mbuf chain. > > > All these requires lots of work and need a real hardware. > > > Oleg, if you have hardware, would you fix it? > > > > Unfortunately i don't have vge hardware. > > -- > Regards, > Pyun YongHyeon -- Oleg. ================================================================ === Oleg Bulyzhin -- OBUL-RIPN -- OBUL-RIPE -- oleg@rinet.ru === ================================================================
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060824004225.GB25876>