Re: Algorithm for hotplugging without an event queue (was: Adding PCMCIA support to the kernel tree

[Oliver Neukum <Oliver.Neukum@lrz.uni-muenchen.de>]

> >To a certain extent you can. You cannot hide removal, but you can hide
> >addition. That's enough.
>
> 	You'll have to show me an example of where not hiding addition
> casses trouble.

Anywhere where you do (short form):

if (type_of(dev) = TYPEA)
	init_typeA(dev);
if (type_of(dev) = TYPEB)
	init_typeB(dev);

> 	In the timeline that you listed, the hot plug events occurred
> *after* the hot plug system was initialized (but while the existing
> hardware was being scanned).  The doubt that I originally expressed
> was about the need to queue hotplug events *before* the hot plug
> system was initialized.

Now I see the misunderstanding.
You are right, strictly speaking you can get away without queueing
events before initialisation. But there is a time during initialisation
where you need to queue as the bus must not change while you scan it.

> >You cannot scan a bus without locking it.
>
> 	You have not shown that or even defined it well at this point.

For the same reason you must not add a device while a script is running.

Scanning a bus will look at some level like.
for (i = 0; i<NUMBER_DEVS;i ++) //or equivalent with a list
	dev = dev_list[0];
		if (type_of(dev) = ...
>
> >While you lock the bus what do you do with the events ?
>
> 	While you *scan* the bus, the kernel would continue
> to generate asynchronous calls to /sbin/hotplug, which would
> block on the lock held by the currently running /sbin/hotplug
> in the algorithm that I described.  When the currently
> running /sbin/hotplug finishes, the second /sbin/hotplug
> that is waiting will catch any changes that happend while
> the previous one was running.

That's the problem.
add for dev A - node 0
lock taken
		dev A removed
		add dev B - node 0 reused
init for device of type A
release lock
remove for dev A - node 0

You see, it is too late.
You must not assume that the second script can undo what the first has done.

> 	For the algorithm that I posted, a naming scheme like the
> one used by /proc/bus/usb is sufficient and there is no need to
> do anything more to preserve the order of events.

No there isn't, but you still you must lock and queue events happening while 
the lock is held.

> 	I am not talking about returing -EPERM on open (which should
> fail if a device is no longer present anyhow), I am talking about
> returning -EIO or something similar on already open file descriptors.

Those that are already open are not a problem.
You must reset permissions before the node can be reused.
Or you use unique names which are not supported at all at present.

> >> 	Secondly, there is a way to get this processing right
> >> where necessary without the need to queue events (which can overflow,
> >> and involve maintiaining arbitrary large dynamic data strucutres).
> >> All you need is a "new" flag that the kernel would set when on a device
> >> when it is inserted.  The hot plug code would be called by the kernel
> >> with an argument indicating what device to check, without necessarily
> >> even indicating whether it was a hot plug or a remove event.
> >>
> >> 	userland_hotplug_handler(dev)
> >> 	{
> >> 		// was_plugged_in[dev] is persistent data, perhaps
> >> 		// stored in a file.
> >>
> >> 		acquire_lock(dev);   // Flock some file; could just have
> >> 				     // one global lock.  Whatever.
> >> 		if (was_plugged_in[dev]
> >> 		    && (new[dev] || !is_plugged_in[dev])) {
> >
> >				^ race condition, the condition you check for might change
> >
> >> 			was_plugged_in[dev] = 0;
> >> 			handle_remove_event(dev);
> >> 		}
> >
> >and you may forget a removal event this way, which is bad
> >
> >> 		if (new[dev]) {
> >> 			new[dev] = 0;
> >> 			if (is_plugged_in[dev]) {
> >> 				was_plugged_in[dev] = 1;
> >> 				handle_insert_event(device);
> >> 			}
> >> 		}
> >> 		release_lock(dev);
> >> 	}
>
> 	(Note: I have deleted the line near the bottom that read
> "old_status[dev] = new_status;".  It was left over from a previous
> edit and did not belong in this listing.)
>
> 	The "race condition" that you described is not a problem
> because the kernel only *sets* new[dev], and only does so when
> it detects an insertion, and then spawns a new /sbin/hotplug, which
> will run after the currently running /sbin/hotplug releases its
> lock and will process the new device if the previous /sbin/hotplug
> instance did not already do so.  /sbin/hotplug only *clears* new[dev].
>
> 	If you do not understand, try making a timeline of an example.

That script suffers from the same problem as described above.
In addition you use is_plugged_in, which presumably reflects current status 
and therefore can change under your feet.
Furthermore by using new as a simple flag you can kill a legitimate new.

	Regards
		Oliver

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel