Date: Tue, 22 Oct 2002 10:57:30 +0800 From: "David Xu" <davidxu@freebsd.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: <freebsd-current@freebsd.org> Subject: Re: rl driver lock order reversal Message-ID: <00fe01c27976$c2ef26f0$ef01a8c0@davidwnt> References: <200210220208.g9M28wLS003077@freefall.freebsd.org><20021021.202051.106891660.imp@bsdimp.com><00bd01c27972$5eea20f0$ef01a8c0@davidwnt> <20021021.204058.31316741.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
----- Original Message -----
From: "M. Warner Losh" <imp@bsdimp.com>
To: <davidxu@freebsd.org>
Cc: <freebsd-current@freebsd.org>
Sent: Tuesday, October 22, 2002 10:40 AM
Subject: Re: rl driver lock order reversal
> In message: <00bd01c27972$5eea20f0$ef01a8c0@davidwnt>
> "David Xu" <davidxu@freebsd.org> writes:
> : today's source code
>
> Looks like I've not committed it. :-(. Also looks like p4 told me
> there were no diffs for reasons unknown...
>
> This removes the lock around the attach routine. It isn't needed,
> since we can't possibly be reentered until after we register an ISR.
>
> Warner
>
> --- /dell/imp/FreeBSD/src/sys/pci/if_rl.c Mon Oct 7 00:15:08 2002
> +++ /dell/imp/p4/newcard/src/sys/pci/if_rl.c Fri Oct 18 00:33:53 2002
> @@ -869,7 +869,6 @@
>
> mtx_init(&sc->rl_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK,
> MTX_DEF | MTX_RECURSE);
> - RL_LOCK(sc);
>
> /*
> * Handle power management nonsense.
> @@ -952,18 +951,6 @@
> goto fail;
> }
>
> - error = bus_setup_intr(dev, sc->rl_irq, INTR_TYPE_NET,
> - rl_intr, sc, &sc->rl_intrhand);
> -
> - if (error) {
> - bus_release_resource(dev, SYS_RES_IRQ, 0, sc->rl_irq);
> - bus_release_resource(dev, RL_RES, RL_RID, sc->rl_res);
> - printf("rl%d: couldn't set up irq\n", unit);
> - goto fail;
> - }
> -
> - callout_handle_init(&sc->rl_stat_ch);
> -
> /* Reset the adapter. */
> rl_reset(sc);
> sc->rl_eecmd_read = RL_EECMD_READ_6BIT;
> @@ -999,7 +986,6 @@
> sc->rl_type = RL_8129;
> else {
> printf("rl%d: unknown device ID: %x\n", unit, rl_did);
> - bus_teardown_intr(dev, sc->rl_irq, sc->rl_intrhand);
> bus_release_resource(dev, SYS_RES_IRQ, 0, sc->rl_irq);
> bus_release_resource(dev, RL_RES, RL_RID, sc->rl_res);
> error = ENXIO;
> @@ -1045,7 +1031,6 @@
>
> if (sc->rl_cdata.rl_rx_buf == NULL) {
> printf("rl%d: no memory for list buffers!\n", unit);
> - bus_teardown_intr(dev, sc->rl_irq, sc->rl_intrhand);
> bus_release_resource(dev, SYS_RES_IRQ, 0, sc->rl_irq);
> bus_release_resource(dev, RL_RES, RL_RID, sc->rl_res);
> bus_dma_tag_destroy(sc->rl_tag);
> @@ -1061,7 +1046,6 @@
> if (mii_phy_probe(dev, &sc->rl_miibus,
> rl_ifmedia_upd, rl_ifmedia_sts)) {
> printf("rl%d: MII without any phy!\n", sc->rl_unit);
> - bus_teardown_intr(dev, sc->rl_irq, sc->rl_intrhand);
> bus_release_resource(dev, SYS_RES_IRQ, 0, sc->rl_irq);
> bus_release_resource(dev, RL_RES, RL_RID, sc->rl_res);
> bus_dmamem_free(sc->rl_tag,
> @@ -1089,11 +1073,23 @@
> * Call MI attach routine.
> */
> ether_ifattach(ifp, ETHER_BPF_SUPPORTED);
> - RL_UNLOCK(sc);
> - return(0);
>
> + error = bus_setup_intr(dev, sc->rl_irq, INTR_TYPE_NET,
> + rl_intr, sc, &sc->rl_intrhand);
> +
> + if (error) {
> + printf("rl%d: couldn't set up irq\n", unit);
> + bus_release_resource(dev, SYS_RES_IRQ, 0, sc->rl_irq);
> + bus_release_resource(dev, RL_RES, RL_RID, sc->rl_res);
> + bus_dmamem_free(sc->rl_tag,
> + sc->rl_cdata.rl_rx_buf, sc->rl_cdata.rl_rx_dmamap);
> + bus_dma_tag_destroy(sc->rl_tag);
> + goto fail;
> + }
> +
> + callout_handle_init(&sc->rl_stat_ch);
> + return(0);
> fail:
> - RL_UNLOCK(sc);
> mtx_destroy(&sc->rl_mtx);
> return(error);
> }
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-current" in the body of the message
Thanks, it works fine.
David
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?00fe01c27976$c2ef26f0$ef01a8c0>