From owner-freebsd-arm@FreeBSD.ORG Mon Nov 16 11:06:49 2009 Return-Path: Delivered-To: freebsd-arm@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3A495106568D for ; Mon, 16 Nov 2009 11:06:49 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 288378FC21 for ; Mon, 16 Nov 2009 11:06:49 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id nAGB6nMX011105 for ; Mon, 16 Nov 2009 11:06:49 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id nAGB6mQF011103 for freebsd-arm@FreeBSD.org; Mon, 16 Nov 2009 11:06:48 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 16 Nov 2009 11:06:48 GMT Message-Id: <200911161106.nAGB6mQF011103@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: gnats set sender to owner-bugmaster@FreeBSD.org using -f From: FreeBSD bugmaster To: freebsd-arm@FreeBSD.org Cc: Subject: Current problem reports assigned to freebsd-arm@FreeBSD.org X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Nov 2009 11:06:49 -0000 Note: to view an individual PR, use: http://www.freebsd.org/cgi/query-pr.cgi?pr=(number). The following is a listing of current problems submitted by FreeBSD users. These represent problem reports covering all versions including experimental development code and obsolete releases. S Tracker Resp. Description -------------------------------------------------------------------------------- o arm/134368 arm [patch] nslu2_led driver for the LEDs on the NSLU2 o arm/134338 arm [patch] Lock GPIO accesses on ixp425 o arm/134092 arm [patch] NSLU.hints contains wrong hints for on board n 3 problems total. From owner-freebsd-arm@FreeBSD.ORG Thu Nov 19 16:38:33 2009 Return-Path: Delivered-To: arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C0806106568B for ; Thu, 19 Nov 2009 16:38:33 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 969938FC27 for ; Thu, 19 Nov 2009 16:38:33 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 30BD546B2C for ; Thu, 19 Nov 2009 11:38:33 -0500 (EST) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 7FB6F8A020 for ; Thu, 19 Nov 2009 11:38:32 -0500 (EST) From: John Baldwin To: arm@freebsd.org Date: Thu, 19 Nov 2009 11:22:02 -0500 User-Agent: KMail/1.9.7 MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200911191122.02975.jhb@freebsd.org> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 19 Nov 2009 11:38:32 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Subject: [PATCH] Fix a few nits in ate(4) X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2009 16:38:33 -0000 I ran into this while working on purging if_watchdog/if_timer use from the tree. This patch fixes a few things in ate(4): - Initialize callout before it is used in atestop() during attach. - Fixup detach. By fixing detach, I mean that it calls ether_ifdetach() to remove outside references to the device before shutting down the device. Please test. --- //depot/vendor/freebsd/src/sys/arm/at91/if_ate.c 2009/06/26 11:50:17 +++ //depot/user/jhb/cleanup/sys/arm/at91/if_ate.c 2009/11/17 18:08:42 @@ -75,8 +75,7 @@ /* * Driver-specific flags. */ -#define ATE_FLAG_DETACHING 0x01 -#define ATE_FLAG_MULTICAST 0x02 +#define ATE_FLAG_MULTICAST 0x01 struct ate_softc { @@ -196,6 +195,7 @@ sc = device_get_softc(dev); sc->dev = dev; ATE_LOCK_INIT(sc); + callout_init_mtx(&sc->tick_ch, &sc->sc_mtx, 0); /* * Allocate resources. @@ -233,7 +233,6 @@ ATE_LOCK(sc); atestop(sc); ATE_UNLOCK(sc); - callout_init_mtx(&sc->tick_ch, &sc->sc_mtx, 0); if ((err = ate_get_mac(sc, eaddr)) != 0) { /* @@ -311,12 +309,11 @@ KASSERT(sc != NULL, ("[ate: %d]: sc is NULL", __LINE__)); ifp = sc->ifp; if (device_is_attached(dev)) { + ether_ifdetach(ifp); ATE_LOCK(sc); - sc->flags |= ATE_FLAG_DETACHING; - atestop(sc); + atestop(sc); ATE_UNLOCK(sc); callout_drain(&sc->tick_ch); - ether_ifdetach(ifp); } if (sc->miibus != NULL) { device_delete_child(dev, sc->miibus); @@ -1109,11 +1105,9 @@ & (IFF_PROMISC | IFF_ALLMULTI)) != 0) ate_rxfilter(sc); } else { - if ((sc->flags & ATE_FLAG_DETACHING) == 0) - ateinit_locked(sc); + ateinit_locked(sc); } } else if ((drv_flags & IFF_DRV_RUNNING) != 0) { - ifp->if_drv_flags &= ~IFF_DRV_RUNNING; atestop(sc); } sc->if_flags = flags; -- John Baldwin From owner-freebsd-arm@FreeBSD.ORG Thu Nov 19 21:22:06 2009 Return-Path: Delivered-To: arm@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 49129106566B; Thu, 19 Nov 2009 21:22:06 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id DC4198FC0C; Thu, 19 Nov 2009 21:22:05 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id nAJLIqnW085081; Thu, 19 Nov 2009 14:18:52 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Thu, 19 Nov 2009 14:19:17 -0700 (MST) Message-Id: <20091119.141917.-1843205663.imp@bsdimp.com> To: jhb@FreeBSD.org From: "M. Warner Losh" In-Reply-To: <200911191122.02975.jhb@freebsd.org> References: <200911191122.02975.jhb@freebsd.org> X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: arm@FreeBSD.org Subject: Re: [PATCH] Fix a few nits in ate(4) X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2009 21:22:06 -0000 In message: <200911191122.02975.jhb@freebsd.org> John Baldwin writes: : I ran into this while working on purging if_watchdog/if_timer use from the : tree. This patch fixes a few things in ate(4): : : - Initialize callout before it is used in atestop() during attach. : - Fixup detach. : : By fixing detach, I mean that it calls ether_ifdetach() to remove outside : references to the device before shutting down the device. Please test. OK. I thought the other order was right. See below for one or two concerns.. Warner : --- //depot/vendor/freebsd/src/sys/arm/at91/if_ate.c 2009/06/26 11:50:17 : +++ //depot/user/jhb/cleanup/sys/arm/at91/if_ate.c 2009/11/17 18:08:42 : @@ -75,8 +75,7 @@ : /* : * Driver-specific flags. : */ : -#define ATE_FLAG_DETACHING 0x01 : -#define ATE_FLAG_MULTICAST 0x02 : +#define ATE_FLAG_MULTICAST 0x01 : : struct ate_softc : { : @@ -196,6 +195,7 @@ : sc = device_get_softc(dev); : sc->dev = dev; : ATE_LOCK_INIT(sc); : + callout_init_mtx(&sc->tick_ch, &sc->sc_mtx, 0); : : /* : * Allocate resources. : @@ -233,7 +233,6 @@ : ATE_LOCK(sc); : atestop(sc); : ATE_UNLOCK(sc); : - callout_init_mtx(&sc->tick_ch, &sc->sc_mtx, 0); : : if ((err = ate_get_mac(sc, eaddr)) != 0) { : /* : @@ -311,12 +309,11 @@ : KASSERT(sc != NULL, ("[ate: %d]: sc is NULL", __LINE__)); : ifp = sc->ifp; : if (device_is_attached(dev)) { : + ether_ifdetach(ifp); : ATE_LOCK(sc); : - sc->flags |= ATE_FLAG_DETACHING; : - atestop(sc); : + atestop(sc); : ATE_UNLOCK(sc); : callout_drain(&sc->tick_ch); : - ether_ifdetach(ifp); : } : if (sc->miibus != NULL) { : device_delete_child(dev, sc->miibus); : @@ -1109,11 +1105,9 @@ : & (IFF_PROMISC | IFF_ALLMULTI)) != 0) : ate_rxfilter(sc); : } else { : - if ((sc->flags & ATE_FLAG_DETACHING) == 0) : - ateinit_locked(sc); : + ateinit_locked(sc); Here we reinitialize the device just before we detach it. I put the detaching stuff in to prevent that. With this change, are you saying this routine won't be called, so we don't need this check anymore? : } : } else if ((drv_flags & IFF_DRV_RUNNING) != 0) { : - ifp->if_drv_flags &= ~IFF_DRV_RUNNING; : atestop(sc); : } : sc->if_flags = flags; Why remove this line? I think it is kinda needed. I don't understand. Warner From owner-freebsd-arm@FreeBSD.ORG Thu Nov 19 22:01:38 2009 Return-Path: Delivered-To: arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 069E4106566B for ; Thu, 19 Nov 2009 22:01:38 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id CB8FB8FC17 for ; Thu, 19 Nov 2009 22:01:37 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 7876246B2C; Thu, 19 Nov 2009 17:01:37 -0500 (EST) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id D90B78A020; Thu, 19 Nov 2009 17:01:36 -0500 (EST) From: John Baldwin To: "M. Warner Losh" Date: Thu, 19 Nov 2009 17:01:33 -0500 User-Agent: KMail/1.9.7 References: <200911191122.02975.jhb@freebsd.org> <20091119.141917.-1843205663.imp@bsdimp.com> In-Reply-To: <20091119.141917.-1843205663.imp@bsdimp.com> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200911191701.33852.jhb@freebsd.org> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 19 Nov 2009 17:01:36 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: arm@freebsd.org Subject: Re: [PATCH] Fix a few nits in ate(4) X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2009 22:01:38 -0000 On Thursday 19 November 2009 4:19:17 pm M. Warner Losh wrote: > In message: <200911191122.02975.jhb@freebsd.org> > John Baldwin writes: > : @@ -1109,11 +1105,9 @@ > : & (IFF_PROMISC | IFF_ALLMULTI)) != 0) > : ate_rxfilter(sc); > : } else { > : - if ((sc->flags & ATE_FLAG_DETACHING) == 0) > : - ateinit_locked(sc); > : + ateinit_locked(sc); > > Here we reinitialize the device just before we detach it. I put the > detaching stuff in to prevent that. With this change, are you saying > this routine won't be called, so we don't need this check anymore? Basically, yes. More to the point, the previous order of calling atestop() before ether_ifdetach() opened up a race wherein userland (or bpf_detach() within if_detach()) could cause this to get invoked after atestop() had been called. Now that we call ether_ifdetach() first, we know that neither bpf nor userland is going to mess with the device anymore, and at that time we can safely call atestop(). That now removes the need for doing this check. > : } > : } else if ((drv_flags & IFF_DRV_RUNNING) != 0) { > : - ifp->if_drv_flags &= ~IFF_DRV_RUNNING; > : atestop(sc); > : } > : sc->if_flags = flags; > > Why remove this line? I think it is kinda needed. I don't understand. It is a duplicate of the first line of atestop(). -- John Baldwin