PATCH: Multiprobe sanitizer

PATCH: Multiprobe sanitizer

[Alan Cox]

There are numerous drivers that can use multithreaded probing but having
some kind of global flag as the way to control this makes migration to
threaded probing hard and since it enables it everywhere and is almost
as likely to cause serious pain as holding a clog dance in a minefield.

If we have a pci_driver multithread_probe flag to inherit you can turn
it on for one driver at a time. 

>From playing so far however I think we need a different model at the
device layer which serializes until the called probe function says "ok
you can start another one now". That would need some kind of flag and
semaphore plus a helper function.

Anyway in the absence of that this is a starting point to usefully play
with this stuff

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-rc4-mm1/drivers/pci/pci-driver.c linux-2.6.18-rc4-mm1/drivers/pci/pci-driver.c
--- linux.vanilla-2.6.18-rc4-mm1/drivers/pci/pci-driver.c	2006-08-15 15:40:17.000000000 +0100
+++ linux-2.6.18-rc4-mm1/drivers/pci/pci-driver.c	2006-08-15 17:24:44.000000000 +0100
@@ -432,7 +432,11 @@
 	drv->driver.bus = &pci_bus_type;
 	drv->driver.owner = owner;
 	drv->driver.kobj.ktype = &pci_driver_kobj_type;
-	drv->driver.multithread_probe = pci_multithread_probe;
+	
+	if(pci_multithread_probe)
+		drv->driver.multithread_probe = pci_multithread_probe;
+	else
+		drv->driver.multithread_probe = drv->multithread_probe;
 
 	spin_lock_init(&drv->dynids.lock);
 	INIT_LIST_HEAD(&drv->dynids.list);
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-rc4-mm1/include/linux/pci.h linux-2.6.18-rc4-mm1/include/linux/pci.h
--- linux.vanilla-2.6.18-rc4-mm1/include/linux/pci.h	2006-08-15 15:40:19.000000000 +0100
+++ linux-2.6.18-rc4-mm1/include/linux/pci.h	2006-08-15 16:37:21.000000000 +0100
@@ -351,6 +351,8 @@
 	struct pci_error_handlers *err_handler;
 	struct device_driver	driver;
 	struct pci_dynids dynids;
+	
+	int multithread_probe;
 };
 
 #define	to_pci_driver(drv) container_of(drv,struct pci_driver, driver)

Re: PATCH: Multiprobe sanitizer

[Greg KH]

On Wed, Aug 16, 2006 at 05:42:18PM +0100, Alan Cox wrote:
> There are numerous drivers that can use multithreaded probing but having
> some kind of global flag as the way to control this makes migration to
> threaded probing hard and since it enables it everywhere and is almost
> as likely to cause serious pain as holding a clog dance in a minefield.
> 
> If we have a pci_driver multithread_probe flag to inherit you can turn
> it on for one driver at a time. 

I was thinking about this originally, but didn't want to go and modify
every PCI driver to enable it :)

But I do like your patch below that lets the options mix nicely.

> From playing so far however I think we need a different model at the
> device layer which serializes until the called probe function says "ok
> you can start another one now". That would need some kind of flag and
> semaphore plus a helper function.

What would this help out with?  Would the PCI layer (for example) handle
this "notify the core that it can continue" type logic?  Or would the
individual drivers need to be able to control it?

I'm guessing that you are thinking of this in relation to the disk
drivers, have you found cases where something like this is necessary due
to hardware constraints?

thanks,

greg k-h

Re: PATCH: Multiprobe sanitizer

[Alan Cox]

Ar Mer, 2006-08-16 am 15:26 -0700, ysgrifennodd Greg KH:
> What would this help out with?  Would the PCI layer (for example) handle
> this "notify the core that it can continue" type logic?  Or would the
> individual drivers need to be able to control it?
> 
> I'm guessing that you are thinking of this in relation to the disk
> drivers, have you found cases where something like this is necessary due
> to hardware constraints?

Actually it occurs everywhere because what happens is

	PCI enumerates in bus order
	Threads *usually* run in bus order

so every n'th boot your devices re-order themselves out of bus order,
and eth1 becomes eth0 for the day.

If you have a "ok now continue scanning" API then we can do

	Grab resources
	Register driver
	Go parallel
	[Slow stuff]

I was thinking if we set multithread = 2 (and define some constants)
then the core code would do

	if (multithread == WAIT)
		down(&drv->wait);


and we'd have

	pci_driver_continue_enumerating(struct pci_driver *drv) {
		up(&drv->wait);
	}

Re: PATCH: Multiprobe sanitizer

[Benjamin Herrenschmidt]

On Thu, 2006-08-17 at 01:36 +0100, Alan Cox wrote:
> Ar Mer, 2006-08-16 am 15:26 -0700, ysgrifennodd Greg KH:
> > What would this help out with?  Would the PCI layer (for example) handle
> > this "notify the core that it can continue" type logic?  Or would the
> > individual drivers need to be able to control it?
> > 
> > I'm guessing that you are thinking of this in relation to the disk
> > drivers, have you found cases where something like this is necessary due
> > to hardware constraints?
> 
> Actually it occurs everywhere because what happens is
> 
> 	PCI enumerates in bus order
> 	Threads *usually* run in bus order
> 
> so every n'th boot your devices re-order themselves out of bus order,
> and eth1 becomes eth0 for the day.

Devices reorder themselves anyways .... look at my XPC shuttle, it has
some usb all-sort-of-card reader built-in and every other day, I get
that to be sda instead of the internal SATA... solution: use mounting by
label. With USB network type of things etc... same problem. I really
don't like the idea of having to add special things in PCI drivers to
"work around" the problem (which will only work in some cases and will
slightly lower how much parallelism we can do).

In fact, I'm all about making the problem worse by agressively
paralellilizing everything to get distros config mecanisms to catch up
and stop using the interface name (or use ifrename).

Ben.

Re: PATCH: Multiprobe sanitizer

[Arjan van de Ven]

> In fact, I'm all about making the problem worse by agressively
> paralellilizing everything to get distros config mecanisms to catch up
> and stop using the interface name (or use ifrename).

distros do this already based on MAC address ;)
at least some do.

-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com

Re: PATCH: Multiprobe sanitizer

[Alan Cox]

Ar Iau, 2006-08-17 am 08:57 +0200, ysgrifennodd Benjamin Herrenschmidt:
> In fact, I'm all about making the problem worse by agressively
> paralellilizing everything to get distros config mecanisms to catch up
> and stop using the interface name (or use ifrename).

I'm so glad I don't have to depend on your code then because I have to
actually deal with the real world not try to commit design suicide in
pursuit in of elegance.

There are numerous cases where bus stability happens to matter because
you cannot identify the different devices. The disk and ethernet cases
are relatively managable (disk has some distribution issues with fstab
etc on older setups). Things get nasty when you look at say sound or
video.

A classic example would be a typical security system with four identical
PCI video capture cards. There is no way other than PCI bus ordering to
order them, and if you don't order them your system isn't useful as you
do different processing on different camera streams.

>From a performance perspective the only one we really care about is
probably disks. Even there we need some kind of ordering (or ability to
order) so that we can handle unlabelled (eg new) volumes and md
components.

Right now lvm/dm/md all depend on real disk names to be useful on large
systems because the time to scan for labels is too high. On small
systems some tools work ok although not all with labels.

Disk has another awkward problem too - power control and management.

Re: PATCH: Multiprobe sanitizer

[Benjamin Herrenschmidt]

On Thu, 2006-08-17 at 09:41 +0100, Alan Cox wrote:
> Ar Iau, 2006-08-17 am 08:57 +0200, ysgrifennodd Benjamin Herrenschmidt:
> > In fact, I'm all about making the problem worse by agressively
> > paralellilizing everything to get distros config mecanisms to catch up
> > and stop using the interface name (or use ifrename).
> 
> I'm so glad I don't have to depend on your code then because I have to
> actually deal with the real world not try to commit design suicide in
> pursuit in of elegance.

Heh, ok, I wasn't 100% serious there :) Though I still think it's a bit
sad to have to add that sort of "workarounds" all over the place to get
some kind of parallelism. At least disk is getting better on most
distros (with, iirc, the notable exception of debian (and maybe ubuntu)
which still defaults to not using labels).

> There are numerous cases where bus stability happens to matter because
> you cannot identify the different devices. The disk and ethernet cases
> are relatively managable (disk has some distribution issues with fstab
> etc on older setups). Things get nasty when you look at say sound or
> video.
> 
> A classic example would be a typical security system with four identical
> PCI video capture cards. There is no way other than PCI bus ordering to
> order them, and if you don't order them your system isn't useful as you
> do different processing on different camera streams.

There are ways. Using sysfs, you can match a given card to a given PCI
"location", in absence of any other way of identifying, it's still
better than relying on driver probe ordering imho.

My main point is if we don't find a way to get some incentive to get it
fixed, userland will probably not be fixed. Maybe your approach as a
"temporary measure" coupled with a kernel command line argument to force
full parallelism is the way to go...

> From a performance perspective the only one we really care about is
> probably disks. Even there we need some kind of ordering (or ability to
> order) so that we can handle unlabelled (eg new) volumes and md
> components.
> 
> Right now lvm/dm/md all depend on real disk names to be useful on large
> systems because the time to scan for labels is too high. On small
> systems some tools work ok although not all with labels.
> 
> Disk has another awkward problem too - power control and management.

I've been thinking about this problem in the past and one solution I
found was to have a concept of a "ID" string (sort-of an exension of the
current syfs busid, though a sysfs path would probably do the trick for
most devices) that allows to uniquely identify a device in the system
with good stability. Devices that can support some kind of serial
number / uuid would be able to override that with it to provide even
better stability. The idea is to provide the stablest possible
identifier for a device, slot ID being fairly unperfect but probably the
best one can do in absence of better.

Probe ordering is fragile and completely defeated with busses that are
already probed asynchronously (like USB or firewire), and things can
only get worse. Thus we need to look for generic solutions, the trick of
maintaining probe ordering will work around problems today but we'll
still hit the wall in an increasing number of cases in the future.

Ben.

Re: PATCH: Multiprobe sanitizer

[Greg KH]

On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> Probe ordering is fragile and completely defeated with busses that are
> already probed asynchronously (like USB or firewire), and things can
> only get worse. Thus we need to look for generic solutions, the trick of
> maintaining probe ordering will work around problems today but we'll
> still hit the wall in an increasing number of cases in the future.

That's exactly why udev was created :)

It can handle bus ordering issues already today just fine, and distros
use it this way in shipping, "enterprise ready" products.

thanks,

greg k-h

Re: PATCH: Multiprobe sanitizer

[Benjamin Herrenschmidt]

On Thu, 2006-08-17 at 05:00 -0700, Greg KH wrote:
> On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> > Probe ordering is fragile and completely defeated with busses that are
> > already probed asynchronously (like USB or firewire), and things can
> > only get worse. Thus we need to look for generic solutions, the trick of
> > maintaining probe ordering will work around problems today but we'll
> > still hit the wall in an increasing number of cases in the future.
> 
> That's exactly why udev was created :)
> 
> It can handle bus ordering issues already today just fine, and distros
> use it this way in shipping, "enterprise ready" products.

Only up to a certain point and for certain drivers... but yeah. That's
probably the right direction to take. Now, I'll let you and Alan argue
wether it's sufficient or not to move toward a fully parallel probing :)

Ben.

Re: PATCH: Multiprobe sanitizer

[Greg KH]

On Thu, Aug 17, 2006 at 02:12:57PM +0200, Benjamin Herrenschmidt wrote:
> On Thu, 2006-08-17 at 05:00 -0700, Greg KH wrote:
> > On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> > > Probe ordering is fragile and completely defeated with busses that are
> > > already probed asynchronously (like USB or firewire), and things can
> > > only get worse. Thus we need to look for generic solutions, the trick of
> > > maintaining probe ordering will work around problems today but we'll
> > > still hit the wall in an increasing number of cases in the future.
> > 
> > That's exactly why udev was created :)
> > 
> > It can handle bus ordering issues already today just fine, and distros
> > use it this way in shipping, "enterprise ready" products.
> 
> Only up to a certain point and for certain drivers... but yeah. 

What drivers are not supported by this?  Seriously, have we missed any?

thanks,

greg k-h

Re: PATCH: Multiprobe sanitizer

[Benjamin Herrenschmidt]

On Thu, 2006-08-17 at 05:22 -0700, Greg KH wrote:
> On Thu, Aug 17, 2006 at 02:12:57PM +0200, Benjamin Herrenschmidt wrote:
> > On Thu, 2006-08-17 at 05:00 -0700, Greg KH wrote:
> > > On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> > > > Probe ordering is fragile and completely defeated with busses that are
> > > > already probed asynchronously (like USB or firewire), and things can
> > > > only get worse. Thus we need to look for generic solutions, the trick of
> > > > maintaining probe ordering will work around problems today but we'll
> > > > still hit the wall in an increasing number of cases in the future.
> > > 
> > > That's exactly why udev was created :)
> > > 
> > > It can handle bus ordering issues already today just fine, and distros
> > > use it this way in shipping, "enterprise ready" products.
> > 
> > Only up to a certain point and for certain drivers... but yeah. 
> 
> What drivers are not supported by this?  Seriously, have we missed any?

udev will not create stable names for a bunch of things... at least not
with the default config that comes with distros. On my shuttle with the
built-in USB card reader, whatever config comes up with the box will
cause the machine to boot or fail to boot due to sda not beeing what
it's expected to be, and udev is of no help because it won't create
stable device names. Yes, I'm confident it _can_ be configured to do so.
It generally is not.

fbdev's also come to mind etc...

It's mostly a distro problem at this point...

Ben.

Re: PATCH: Multiprobe sanitizer

[Arjan van de Ven]

On Thu, 2006-08-17 at 14:37 +0200, Benjamin Herrenschmidt wrote:
> On Thu, 2006-08-17 at 05:22 -0700, Greg KH wrote:
> > On Thu, Aug 17, 2006 at 02:12:57PM +0200, Benjamin Herrenschmidt wrote:
> > > On Thu, 2006-08-17 at 05:00 -0700, Greg KH wrote:
> > > > On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> > > > > Probe ordering is fragile and completely defeated with busses that are
> > > > > already probed asynchronously (like USB or firewire), and things can
> > > > > only get worse. Thus we need to look for generic solutions, the trick of
> > > > > maintaining probe ordering will work around problems today but we'll
> > > > > still hit the wall in an increasing number of cases in the future.
> > > > 
> > > > That's exactly why udev was created :)
> > > > 
> > > > It can handle bus ordering issues already today just fine, and distros
> > > > use it this way in shipping, "enterprise ready" products.
> > > 
> > > Only up to a certain point and for certain drivers... but yeah. 
> > 
> > What drivers are not supported by this?  Seriously, have we missed any?
> 
> udev will not create stable names for a bunch of things... at least not
> with the default config that comes with distros. On my shuttle with the
> built-in USB card reader, whatever config comes up with the box will
> cause the machine to boot or fail to boot due to sda not beeing what
> it's expected to be, and udev is of no help because it won't create
> stable device names. 

that's what mount by label is for though..

(which isn't a udev but a distro thing)

Re: PATCH: Multiprobe sanitizer

[Benjamin Herrenschmidt]

On Thu, 2006-08-17 at 15:04 +0200, Arjan van de Ven wrote:

> > udev will not create stable names for a bunch of things... at least not
> > with the default config that comes with distros. On my shuttle with the
> > built-in USB card reader, whatever config comes up with the box will
> > cause the machine to boot or fail to boot due to sda not beeing what
> > it's expected to be, and udev is of no help because it won't create
> > stable device names. 
> 
> that's what mount by label is for though..
> 
> (which isn't a udev but a distro thing)

I know and that's what I use, but it wasn't standard on my distro
(debian, ok, my fault...) and the problem is still there for pretty much
any other class of device in the system... (and mount by label isn't a
udev thing.. won't fix anything but mountable devices which is fairly
limitative, no help with usb vs. firewire vs. ata cd-writers etc....
udev can do it but isn't configured to do so by default on distros)

Ben.

Re: PATCH: Multiprobe sanitizer

[Greg KH]

On Thu, Aug 17, 2006 at 03:04:21PM +0200, Arjan van de Ven wrote:
> On Thu, 2006-08-17 at 14:37 +0200, Benjamin Herrenschmidt wrote:
> > On Thu, 2006-08-17 at 05:22 -0700, Greg KH wrote:
> > > On Thu, Aug 17, 2006 at 02:12:57PM +0200, Benjamin Herrenschmidt wrote:
> > > > On Thu, 2006-08-17 at 05:00 -0700, Greg KH wrote:
> > > > > On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> > > > > > Probe ordering is fragile and completely defeated with busses that are
> > > > > > already probed asynchronously (like USB or firewire), and things can
> > > > > > only get worse. Thus we need to look for generic solutions, the trick of
> > > > > > maintaining probe ordering will work around problems today but we'll
> > > > > > still hit the wall in an increasing number of cases in the future.
> > > > > 
> > > > > That's exactly why udev was created :)
> > > > > 
> > > > > It can handle bus ordering issues already today just fine, and distros
> > > > > use it this way in shipping, "enterprise ready" products.
> > > > 
> > > > Only up to a certain point and for certain drivers... but yeah. 
> > > 
> > > What drivers are not supported by this?  Seriously, have we missed any?
> > 
> > udev will not create stable names for a bunch of things... at least not
> > with the default config that comes with distros. On my shuttle with the
> > built-in USB card reader, whatever config comes up with the box will
> > cause the machine to boot or fail to boot due to sda not beeing what
> > it's expected to be, and udev is of no help because it won't create
> > stable device names. 
> 
> that's what mount by label is for though..
> 
> (which isn't a udev but a distro thing)

No, it's a udev thing too, look in /dev/disk/by-label/ and use that in
your fstab.

So yes, udev already handles this for block devices and input devices.
It is simple to add new rules for other subsystems as people find that
they need them.

thanks,

greg k-h

Re: PATCH: Multiprobe sanitizer

[Olaf Hering]

On Thu, Aug 17, 2006 at 05:22:44AM -0700, Greg KH wrote:
> On Thu, Aug 17, 2006 at 02:12:57PM +0200, Benjamin Herrenschmidt wrote:
> > On Thu, 2006-08-17 at 05:00 -0700, Greg KH wrote:
> > > On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> > > > Probe ordering is fragile and completely defeated with busses that are
> > > > already probed asynchronously (like USB or firewire), and things can
> > > > only get worse. Thus we need to look for generic solutions, the trick of
> > > > maintaining probe ordering will work around problems today but we'll
> > > > still hit the wall in an increasing number of cases in the future.
> > > 
> > > That's exactly why udev was created :)
> > > 
> > > It can handle bus ordering issues already today just fine, and distros
> > > use it this way in shipping, "enterprise ready" products.
> > 
> > Only up to a certain point and for certain drivers... but yeah. 
> 
> What drivers are not supported by this?  Seriously, have we missed any?

Do serial drivers have a device symlink now, and video drivers?

Re: PATCH: Multiprobe sanitizer

[Greg KH]

On Thu, Aug 17, 2006 at 04:43:29PM +0200, Olaf Hering wrote:
> On Thu, Aug 17, 2006 at 05:22:44AM -0700, Greg KH wrote:
> > On Thu, Aug 17, 2006 at 02:12:57PM +0200, Benjamin Herrenschmidt wrote:
> > > On Thu, 2006-08-17 at 05:00 -0700, Greg KH wrote:
> > > > On Thu, Aug 17, 2006 at 11:24:35AM +0200, Benjamin Herrenschmidt wrote:
> > > > > Probe ordering is fragile and completely defeated with busses that are
> > > > > already probed asynchronously (like USB or firewire), and things can
> > > > > only get worse. Thus we need to look for generic solutions, the trick of
> > > > > maintaining probe ordering will work around problems today but we'll
> > > > > still hit the wall in an increasing number of cases in the future.
> > > > 
> > > > That's exactly why udev was created :)
> > > > 
> > > > It can handle bus ordering issues already today just fine, and distros
> > > > use it this way in shipping, "enterprise ready" products.
> > > 
> > > Only up to a certain point and for certain drivers... but yeah. 
> > 
> > What drivers are not supported by this?  Seriously, have we missed any?
> 
> Do serial drivers have a device symlink now

Yes:
 $ tree /sys/class/tty/ttyS0
/sys/class/tty/ttyS0
|-- dev
|-- device -> ../../../devices/platform/serial8250
|-- subsystem -> ../../../class/tty
`-- uevent

> and video drivers?

Properly written ones do, I have heard reports that some do not, but
that's a driver bug that should be fixed with a one line addition.  If
you know of any specific ones, please let me know and I'll make the
needed change.

thanks,

greg k-h

Re: PATCH: Multiprobe sanitizer

[Greg KH]

On Thu, Aug 17, 2006 at 01:36:33AM +0100, Alan Cox wrote:
> Ar Mer, 2006-08-16 am 15:26 -0700, ysgrifennodd Greg KH:
> > What would this help out with?  Would the PCI layer (for example) handle
> > this "notify the core that it can continue" type logic?  Or would the
> > individual drivers need to be able to control it?
> > 
> > I'm guessing that you are thinking of this in relation to the disk
> > drivers, have you found cases where something like this is necessary due
> > to hardware constraints?
> 
> Actually it occurs everywhere because what happens is
> 
> 	PCI enumerates in bus order
> 	Threads *usually* run in bus order
> 
> so every n'th boot your devices re-order themselves out of bus order,
> and eth1 becomes eth0 for the day.

As per the help information for this option, this will happen.  Happens
all the time on my machines already, that's why I use a tool that always
names the devices in the proper way (like almost all distros do these
days.)

It's a trade off, if you want a parallel device probe, you have to stop
relying on the kernel name for devices and use something that is not
going to change.  Like PCI device ids, (well, ok, they change all the time
on pci hotplug boxes and bios upgrades) or MAC addresses, or serial
numbers, or file system labels.

> If you have a "ok now continue scanning" API then we can do
> 
> 	Grab resources
> 	Register driver
> 	Go parallel
> 	[Slow stuff]

It seems that for some types of devices, the "Grab resources" portion
takes the longest (SCSI...).  And the "register driver" stuff already
happens later in the process, in an async manner today.  So that's not
going to work :(

> I was thinking if we set multithread = 2 (and define some constants)
> then the core code would do
> 
> 	if (multithread == WAIT)
> 		down(&drv->wait);
> 
> 
> and we'd have
> 
> 	pci_driver_continue_enumerating(struct pci_driver *drv) {
> 		up(&drv->wait);
> 	}

Yeah, I played with limiting the number of outstanding threads in a
manner like this, but couldn't come up with a real reason to limit it.

If you can think of a place in the PCI core that we could block on in
this manner, please let me know.

thanks,

greg k-h