Re: ppb(4): free dynamically allocated bus
> Date: Tue, 16 Apr 2019 16:39:42 +0200
> From: Patrick Wildt
>
> On Tue, Apr 16, 2019 at 03:23:37PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 15 Apr 2019 08:50:04 +0200
> > > From: Patrick Wildt
> > > Content-Type: text/plain; charset="utf-8"
> > > Content-Disposition: inline
> > >
> > > Hi,
> > >
> > > with kettenis' change from 2015(?) we are able to configure ppb(4)'s
> > > that are hotplugged. I think on detach we should make sure to free the
> > > bus range that was allocated for that device dynamically. Otherwise
> > > plug and pull can starve the extent.
> > >
> > > Feedback?
> >
> > The only thing I can think of is that the name sc_pbusex is a
> > bitconfusing since we already have sc_pmemex where the "p" stands for
> > prefetchable. Maybe just call it sc_parent_busex?
>
> Heh, true, you're right. That better? ok?
ok kettenis@
Regadring the other ppb(4) diff; I'd like to test that on a system
with ExpressCard first.
> diff --git a/sys/dev/pci/ppb.c b/sys/dev/pci/ppb.c
> index 34e3697dd51..b4894926bd0 100644
> --- a/sys/dev/pci/ppb.c
> +++ b/sys/dev/pci/ppb.c
> @@ -66,6 +66,7 @@ struct ppb_softc {
> pcitag_t sc_tag;/* ...and tag. */
> pci_intr_handle_t sc_ih[4];
> void *sc_intrhand;
> + struct extent *sc_parent_busex;
> struct extent *sc_busex;
> struct extent *sc_ioex;
> struct extent *sc_memex;
> @@ -77,6 +78,9 @@ struct ppb_softc {
> struct task sc_remove_task;
> struct timeout sc_to;
>
> + u_long sc_busnum;
> + u_long sc_busrange;
> +
> bus_addr_t sc_iobase, sc_iolimit;
> bus_addr_t sc_membase, sc_memlimit;
> bus_addr_t sc_pmembase, sc_pmemlimit;
> @@ -390,6 +394,10 @@ ppbdetach(struct device *self, int flags)
> free(name, M_DEVBUF, PPB_EXNAMLEN);
> }
>
> + if (sc->sc_parent_busex)
> + extent_free(sc->sc_parent_busex, sc->sc_busnum,
> + sc->sc_busrange, EX_NOWAIT);
> +
> return (rv);
> }
>
> @@ -529,6 +537,9 @@ ppb_alloc_busrange(struct ppb_softc *sc, struct
> pci_attach_args *pa,
> if (extent_alloc(pa->pa_busex, busrange, 1, 0, 0,
> EX_NOWAIT, ))
> continue;
> + sc->sc_parent_busex = pa->pa_busex;
> + sc->sc_busnum = busnum;
> + sc->sc_busrange = busrange;
> *busdata |= pa->pa_bus;
> *busdata |= (busnum << 8);
> *busdata |= ((busnum + busrange - 1) << 16);
>
Re: ppb(4): free dynamically allocated bus
On Tue, Apr 16, 2019 at 03:23:37PM +0200, Mark Kettenis wrote:
> > Date: Mon, 15 Apr 2019 08:50:04 +0200
> > From: Patrick Wildt
> > Content-Type: text/plain; charset="utf-8"
> > Content-Disposition: inline
> >
> > Hi,
> >
> > with kettenis' change from 2015(?) we are able to configure ppb(4)'s
> > that are hotplugged. I think on detach we should make sure to free the
> > bus range that was allocated for that device dynamically. Otherwise
> > plug and pull can starve the extent.
> >
> > Feedback?
>
> The only thing I can think of is that the name sc_pbusex is a
> bitconfusing since we already have sc_pmemex where the "p" stands for
> prefetchable. Maybe just call it sc_parent_busex?
Heh, true, you're right. That better? ok?
diff --git a/sys/dev/pci/ppb.c b/sys/dev/pci/ppb.c
index 34e3697dd51..b4894926bd0 100644
--- a/sys/dev/pci/ppb.c
+++ b/sys/dev/pci/ppb.c
@@ -66,6 +66,7 @@ struct ppb_softc {
pcitag_t sc_tag;/* ...and tag. */
pci_intr_handle_t sc_ih[4];
void *sc_intrhand;
+ struct extent *sc_parent_busex;
struct extent *sc_busex;
struct extent *sc_ioex;
struct extent *sc_memex;
@@ -77,6 +78,9 @@ struct ppb_softc {
struct task sc_remove_task;
struct timeout sc_to;
+ u_long sc_busnum;
+ u_long sc_busrange;
+
bus_addr_t sc_iobase, sc_iolimit;
bus_addr_t sc_membase, sc_memlimit;
bus_addr_t sc_pmembase, sc_pmemlimit;
@@ -390,6 +394,10 @@ ppbdetach(struct device *self, int flags)
free(name, M_DEVBUF, PPB_EXNAMLEN);
}
+ if (sc->sc_parent_busex)
+ extent_free(sc->sc_parent_busex, sc->sc_busnum,
+ sc->sc_busrange, EX_NOWAIT);
+
return (rv);
}
@@ -529,6 +537,9 @@ ppb_alloc_busrange(struct ppb_softc *sc, struct
pci_attach_args *pa,
if (extent_alloc(pa->pa_busex, busrange, 1, 0, 0,
EX_NOWAIT, ))
continue;
+ sc->sc_parent_busex = pa->pa_busex;
+ sc->sc_busnum = busnum;
+ sc->sc_busrange = busrange;
*busdata |= pa->pa_bus;
*busdata |= (busnum << 8);
*busdata |= ((busnum + busrange - 1) << 16);
Re: ppb(4): free dynamically allocated bus
> Date: Mon, 15 Apr 2019 08:50:04 +0200
> From: Patrick Wildt
> Content-Type: text/plain; charset="utf-8"
> Content-Disposition: inline
>
> Hi,
>
> with kettenis' change from 2015(?) we are able to configure ppb(4)'s
> that are hotplugged. I think on detach we should make sure to free the
> bus range that was allocated for that device dynamically. Otherwise
> plug and pull can starve the extent.
>
> Feedback?
The only thing I can think of is that the name sc_pbusex is a
bitconfusing since we already have sc_pmemex where the "p" stands for
prefetchable. Maybe just call it sc_parent_busex?
> diff --git a/sys/dev/pci/ppb.c b/sys/dev/pci/ppb.c
> index 34e3697dd51..a9ff32ce33c 100644
> --- a/sys/dev/pci/ppb.c
> +++ b/sys/dev/pci/ppb.c
> @@ -67,6 +67,7 @@ struct ppb_softc {
> pci_intr_handle_t sc_ih[4];
> void *sc_intrhand;
> struct extent *sc_busex;
> + struct extent *sc_pbusex;
> struct extent *sc_ioex;
> struct extent *sc_memex;
> struct extent *sc_pmemex;
> @@ -77,6 +78,9 @@ struct ppb_softc {
> struct task sc_remove_task;
> struct timeout sc_to;
>
> + u_long sc_busnum;
> + u_long sc_busrange;
> +
> bus_addr_t sc_iobase, sc_iolimit;
> bus_addr_t sc_membase, sc_memlimit;
> bus_addr_t sc_pmembase, sc_pmemlimit;
> @@ -390,6 +394,10 @@ ppbdetach(struct device *self, int flags)
> free(name, M_DEVBUF, PPB_EXNAMLEN);
> }
>
> + if (sc->sc_pbusex)
> + extent_free(sc->sc_pbusex, sc->sc_busnum,
> + sc->sc_busrange, EX_NOWAIT);
> +
> return (rv);
> }
>
> @@ -529,6 +537,9 @@ ppb_alloc_busrange(struct ppb_softc *sc, struct
> pci_attach_args *pa,
> if (extent_alloc(pa->pa_busex, busrange, 1, 0, 0,
> EX_NOWAIT, ))
> continue;
> + sc->sc_pbusex = pa->pa_busex;
> + sc->sc_busnum = busnum;
> + sc->sc_busrange = busrange;
> *busdata |= pa->pa_bus;
> *busdata |= (busnum << 8);
> *busdata |= ((busnum + busrange - 1) << 16);
>
>
ppb(4): free dynamically allocated bus
Hi,
with kettenis' change from 2015(?) we are able to configure ppb(4)'s
that are hotplugged. I think on detach we should make sure to free the
bus range that was allocated for that device dynamically. Otherwise
plug and pull can starve the extent.
Feedback?
Patrick
diff --git a/sys/dev/pci/ppb.c b/sys/dev/pci/ppb.c
index 34e3697dd51..a9ff32ce33c 100644
--- a/sys/dev/pci/ppb.c
+++ b/sys/dev/pci/ppb.c
@@ -67,6 +67,7 @@ struct ppb_softc {
pci_intr_handle_t sc_ih[4];
void *sc_intrhand;
struct extent *sc_busex;
+ struct extent *sc_pbusex;
struct extent *sc_ioex;
struct extent *sc_memex;
struct extent *sc_pmemex;
@@ -77,6 +78,9 @@ struct ppb_softc {
struct task sc_remove_task;
struct timeout sc_to;
+ u_long sc_busnum;
+ u_long sc_busrange;
+
bus_addr_t sc_iobase, sc_iolimit;
bus_addr_t sc_membase, sc_memlimit;
bus_addr_t sc_pmembase, sc_pmemlimit;
@@ -390,6 +394,10 @@ ppbdetach(struct device *self, int flags)
free(name, M_DEVBUF, PPB_EXNAMLEN);
}
+ if (sc->sc_pbusex)
+ extent_free(sc->sc_pbusex, sc->sc_busnum,
+ sc->sc_busrange, EX_NOWAIT);
+
return (rv);
}
@@ -529,6 +537,9 @@ ppb_alloc_busrange(struct ppb_softc *sc, struct
pci_attach_args *pa,
if (extent_alloc(pa->pa_busex, busrange, 1, 0, 0,
EX_NOWAIT, ))
continue;
+ sc->sc_pbusex = pa->pa_busex;
+ sc->sc_busnum = busnum;
+ sc->sc_busrange = busrange;
*busdata |= pa->pa_bus;
*busdata |= (busnum << 8);
*busdata |= ((busnum + busrange - 1) << 16);
