diff options
| author | riastradh <riastradh@NetBSD.org> | 2021-06-16 00:21:17 +0000 |
|---|---|---|
| committer | riastradh <riastradh@NetBSD.org> | 2021-06-16 00:21:17 +0000 |
| commit | bbdda93be2f0bfefe91466b398fe42700a6ea284 (patch) | |
| tree | e75e77046575209dbb21f73d0bc44c61ec0ec6c0 /sys/dev/ic/dwc_gmac.c | |
| parent | 5a96957fa793651c8e7093a5a4c2fcae87232af2 (diff) | |
if_attach and if_initialize cannot fail, don't test return value
These were originally made failable back in 2017 when if_initialize
allocated a softint in every interface for link state changes, so
that it could fail gracefully instead of panicking:
https://mail-index.NetBSD.org/source-changes/2017/10/23/msg089053.html
However, this spawned many seldom- or never-tested error branches,
which are risky to have around. And that softint in every interface
has since been replaced by a single global workqueue, because link
state changes require thread context but not low latency or high
throughput:
https://mail-index.NetBSD.org/source-changes/2020/02/06/msg113759.html
So there is no longer any reason for if_initialize to fail. (The
subroutine if_stats_init can't fail because percpu_alloc can't fail
either.)
There is a snag: the softint_establish in if_percpuq_create could
fail, potentially leading to bad consequences later on trying to use
the softint. This change doesn't introduce any new bugs because of
the snag -- if_percpuq_attach was already broken. However, the snag
can be better addressed without spawning error branches, either by
using a single softint or making softints less scarce.
(Separate commit will change the signatures of if_attach and
if_initialize to return void, scheduled to ride whatever is the next
convenient kernel bump.)
Patch and testing on amd64 and evbmips64-eb by maya@; commit message
soliloquy, and compile-testing on evbppc/i386/earmv7hf, by me.
Diffstat (limited to 'sys/dev/ic/dwc_gmac.c')
| -rw-r--r-- | sys/dev/ic/dwc_gmac.c | 15 |
1 files changed, 3 insertions, 12 deletions
diff --git a/sys/dev/ic/dwc_gmac.c b/sys/dev/ic/dwc_gmac.c index e12d86a8ffa..1d2a7fa50a0 100644 --- a/sys/dev/ic/dwc_gmac.c +++ b/sys/dev/ic/dwc_gmac.c @@ -1,4 +1,4 @@ -/* $NetBSD: dwc_gmac.c,v 1.73 2021/05/13 05:56:39 msaitoh Exp $ */ +/* $NetBSD: dwc_gmac.c,v 1.74 2021/06/16 00:21:18 riastradh Exp $ */ /*- * Copyright (c) 2013, 2014 The NetBSD Foundation, Inc. @@ -41,7 +41,7 @@ #include <sys/cdefs.h> -__KERNEL_RCSID(1, "$NetBSD: dwc_gmac.c,v 1.73 2021/05/13 05:56:39 msaitoh Exp $"); +__KERNEL_RCSID(1, "$NetBSD: dwc_gmac.c,v 1.74 2021/06/16 00:21:18 riastradh Exp $"); /* #define DWC_GMAC_DEBUG 1 */ @@ -188,7 +188,6 @@ dwc_gmac_attach(struct dwc_gmac_softc *sc, int phy_id, uint32_t mii_clk) struct mii_data * const mii = &sc->sc_mii; struct ifnet * const ifp = &sc->sc_ec.ec_if; prop_dictionary_t dict; - int rv; mutex_init(&sc->sc_mdio_lock, MUTEX_DEFAULT, IPL_NET); sc->sc_mii_clk = mii_clk & 7; @@ -332,9 +331,7 @@ dwc_gmac_attach(struct dwc_gmac_softc *sc, int phy_id, uint32_t mii_clk) * Ready, attach interface */ /* Attach the interface. */ - rv = if_initialize(ifp); - if (rv != 0) - goto fail_2; + if_initialize(ifp); sc->sc_ipq = if_percpuq_create(&sc->sc_ec.ec_if); if_deferred_start_init(ifp, NULL); ether_ifattach(ifp, enaddr); @@ -355,12 +352,6 @@ dwc_gmac_attach(struct dwc_gmac_softc *sc, int phy_id, uint32_t mii_clk) return 0; -fail_2: - ifmedia_removeall(&mii->mii_media); - mii_detach(mii, MII_PHY_ANY, MII_OFFSET_ANY); - mutex_destroy(&sc->sc_txq.t_mtx); - mutex_destroy(&sc->sc_rxq.r_mtx); - mutex_obj_free(sc->sc_lock); fail: dwc_gmac_free_rx_ring(sc, &sc->sc_rxq); dwc_gmac_free_tx_ring(sc, &sc->sc_txq); |
