[dpdk-dev] [PATCH] pci: save list of detached devices, and re-probe during driver unload
Raz Amir
razamir22 at gmail.com
Wed Mar 4 11:28:01 CET 2015
Understood.
I already sent the updated patch, so I will fix this and resend it soon.
-----Original Message-----
From: Bruce Richardson [mailto:bruce.richardson at intel.com]
Sent: 04 March 2015 12:13
To: Raz Amir
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices, and
re-probe during driver unload
On Wed, Mar 04, 2015 at 11:07:41AM +0200, Raz Amir wrote:
> Thank you.
>
> See answers inline (mostly ack, but not only), and I will send the
> updated patch soon.
>
>
>
> > -----Original Message-----
>
> > From: Bruce Richardson [mailto:bruce.richardson at intel.com]
>
> > Sent: 03 March 2015 15:33
>
> > To: Raz Amir
>
> > Cc: dev at dpdk.org
>
> > Subject: Re: [dpdk-dev] [PATCH] pci: save list of detached devices,
> > and
> re-
>
> > probe during driver unload
>
> >
>
> > On Thu, Feb 26, 2015 at 06:33:20AM +0000, Raz Amir wrote:
>
> > > Added code that saves the pointers to the detached devices, during
>
> > > driver loading, and during driver unloading, go over the list, and
>
> > > re-attach them by calling device_probe_and_attach on each device.
>
> > >
>
> > > Signed-off-by: Raz Amir < <mailto:razamir22 at gmail.com>
> razamir22 at gmail.com>
>
> >
>
> > Couple of minor comments below. Otherwise all looks good to me.
>
> >
>
> > Acked-by: Bruce Richardson < <mailto:bruce.richardson at intel.com>
> bruce.richardson at intel.com>
>
> > > ---
>
> > > lib/librte_eal/bsdapp/nic_uio/nic_uio.c | 26
>
> > > +++++++++++++++++++++++++-
>
> > > 1 file changed, 25 insertions(+), 1 deletion(-)
>
> > >
>
> > > diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>
> > > b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>
> > > index 5ae8560..7d702a5 100644
>
> > > --- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>
> > > +++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
>
> > > @@ -55,6 +55,9 @@ __FBSDID("$FreeBSD$");
>
> > >
>
> > > #define MAX_BARS (PCIR_MAX_BAR_0 + 1)
>
> > >
>
> > > +#define MAX_DETACHED_DEVICES 128
>
> > > +static device_t detached_devices[MAX_DETACHED_DEVICES] = {};
> > > +static
>
> > > +int last_detached = 0;
>
> > Maybe num_detached/nb_detached or even just "detached" instead of
>
> > "last_detached".
>
> Ack.
>
>
>
> >
>
> > >
>
> > > struct nic_uio_softc {
>
> > > device_t dev_t;
>
> > > @@ -291,14 +294,35 @@ nic_uio_load(void)
>
> > > if (dev != NULL)
>
> >
>
> > We are getting into some serious levels of indentation below, so
> > maybe
> flip
>
> > this condition around and put in a "continue" instead, so that we
> > can
> dedent
>
> > everything below that follows it.
>
> >
>
> Ack.
>
>
>
> > > for (i = 0; i <
> > > NUM_DEVICES;
> i++)
>
> > > if
> (pci_get_vendor(dev) == devices[i].vend
>
> > &&
>
> > > -
> pci_get_device(dev) ==
>
> > devices[i].dev)
>
> > > +
> pci_get_device(dev) ==
>
> > devices[i].dev) {
>
> > > +
> if (last_detached+1 <
>
> > MAX_DETACHED_DEVICES) {
>
> > I don't think you need the +1 here.
>
> It is needed, otherwise the last object will be added at
> MAX_DETACHED_DEVICES position while the last position is
> MAX_DETACHED_DEVICES-1.
Yes, the last position is MAX_DETACHED_DEVICES-1, but you do the addition of
the element to the array using "detached_devices[last_detached++]", i.e. a
post-increment, so when last_detached == (MAX_DETACHED_DEVICES-1), you still
can fill in an entry. Next time around, when last_detached ==
MAX_DETACHED_DEVICES it's no longer safe to add, and the condition
"last_detached < MAX_DETACHED_DEVICES) will now fail. No +1 or -1 necessary
to prevent this.
/Bruce
More information about the dev
mailing list