From owner-freebsd-current@FreeBSD.ORG Fri Aug 25 14:24:54 2006 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id CAFC716A66A; Fri, 25 Aug 2006 14:24:54 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by mx1.FreeBSD.org (Postfix) with ESMTP id F008843D78; Fri, 25 Aug 2006 14:24:44 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from zion.baldwin.cx (zion.baldwin.cx [192.168.0.7]) (authenticated bits=0) by server.baldwin.cx (8.13.6/8.13.6) with ESMTP id k7PEOXjb075845; Fri, 25 Aug 2006 10:24:42 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-current@freebsd.org Date: Fri, 25 Aug 2006 10:23:59 -0400 User-Agent: KMail/1.9.1 References: <20060822073201.GI12848@cdnetworks.co.kr> <20060824014121.GE22634@cdnetworks.co.kr> <20060824192621.GB39797@lath.rinet.ru> In-Reply-To: <20060824192621.GB39797@lath.rinet.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200608251024.00401.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [192.168.0.1]); Fri, 25 Aug 2006 10:24:42 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/1728/Fri Aug 25 01:55:58 2006 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Pyun YongHyeon , Oleg Bulyzhin Subject: Re: call for bge(4) testers X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Aug 2006 14:24:55 -0000 On Thursday 24 August 2006 15:26, Oleg Bulyzhin wrote: > 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. Agreed. This is much more sane than trying to pass the locks around. -- John Baldwin