more eeh

[Greg KH]

On Fri, Mar 19, 2004 at 11:32:29AM +1100, Paul Mackerras wrote:
> > > I was thinking that the unplug event generation, resetting and
> > > reconnecting of the device, and plug event generation would be done by
> > > a kernel thread.  I don't think we want to rely on userspace for that,
> > > because userspace may get blocked while the device is gone.
> >
> > But you want userspace to do this.  There are systems with a few
> > different PCI Hotplug controller drivers on them.  The different
> > controller drivers control different slots.  Userspace is the only place
> > that can reliably handle this.
>
> I don't understand this; surely if you get a pointer to a pci_dev you
> can get to pci_dev->driver->remove and call that, can't you?

No, you need to call pci_remove_bus_device() for that device so that the
pci core properly cleans up after your device is gone.  That's what the
pci hotplug controller drivers eventually call, after handling a bunch
of other housekeeping that their hardware requires (putting resources
back into a pool, etc.)

> Or are you saying that there is no consistent API to the drivers for
> the hot-plug capable PCI bridges?

There is.  The drivers see the remove() call.  The pci core needs to be
called with pci_remove_bus_device().  It's the pci hotplug controller
drivers that don't have a "consistant" interface yet, as I've never
thought to expose one.  It wouldn't be that hard to do that in the pci
hotplug core if you really think it's necessary.  But realize that there
is no universal way to get from a pci device to the pci slot that it is
associated with in order to turn it off through the pci hotplug core.
That's probably going to be your main stumbling block.

> > And if you are a kernel thread, you would have the same issues that
> > dropping to userspace and doing the disconnect there causes.
>
> Not all of the same issues; a kernel thread doesn't have to worry
> about its code or data being paged out, for instance.

Good point, I didn't think of that.

> Well, I would be concerned about the maximum latency, not the average
> latency.  I accept that the average would be milliseconds, but the
> maximum could be tens of seconds on a heavily loaded system, couldn't
> it?  Especially if it involves execing a new process and that requires
> disk I/O.

Yes it could.  But so could execution of a kernel thread, right?  This
isn't a "real time" system by any means :)

> > Now the issue of putting the hotplug script on a disk that just got a
> > error would indicate that you really need a type (a) driver for that
> > kind of thing.
>
> Part of my thinking is that I would like the API for type (a) drivers
> to be an extension of the PCI hotplug API rather than being completely
> disjoint.  In other words, I would like the type (a) driver to get the
> unplug event, and then determine (via a special call, or a parameter
> to the remove() function) that this is an EEH event and therefore the
> absence of the device is likely to be transient.  The driver would
> then not report the removal immediately, but would wait (with a
> timeout) for the device to come back.  When it came back it would
> recognize that this is the same device, reinitialize it and carry on.
> If the device didn't come back shortly, then it would do the normal
> device removal things.

Ok, that would work.  It would take a bunch of driver tweeking, but I'll
leave that to you.

> In any case, whatever the API, we are going to have to have the
> infrastructure in the kernel to do the slot reset and reconnect, for
> type (a) drivers to use.  Type (a) drivers need to be able to recover
> without relying on userspace, obviously.  It doesn't make sense to me
> to have the same logic in two places, in the kernel and in userspace,
> and use one or the other depending on what sort of driver we have.

No, use the logic only in userspace, don't duplicate it in the kernel
too.

Remember, you are asking to put policy about EEH events into the kernel.
Do you really want that?  If you can say that you will _always_ just
want to call release() on the driver, then I'll buy it and it will
remain a PPC64 specific "feature".  If, on the otherhand, you want to do
something that will work for all platforms, I suggest the userspace
hotplug method.  That will work for everyone.

thanks,

greg k-h

** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/