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

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

> >if (type_of(dev) = TYPEA)
> >	init_typeA(dev);
> >if (type_of(dev) = TYPEB)
> >	init_typeB(dev);
>
> 	You have already agreed that you cannot "lock" extraction
> events, which is something that must have occured for device A
> to be replaced by device B.  Queuing hot plug events in the kernel
> and delaying publishing of insert events in /proc would not

I want the device completely inaccessible to user space, not just delay 
publishing.

> change the fact that the typeA device had already been physically
> removed under this scenario.  All that you scheme would do would be
> to delay the correct call to init_typeB (because the typeB device
> had not yet been detected).

If init_typeA were called after device A had been removed it would fail.
If however type_of returned A before device replacement and init_typeA
were called for a device of type B an error would be made.
The wrong method of initialisation would be called resulting in unknown
consequences.

> 	Likewise, under any algorithm, under any "locking" scheme, in
> the case where the user yanks device A a nanosecond after it is
> detected and inserts device B a nanosecond later, you have rely on
> the kernel's handling of those interrupts and init_typeA's error
> handling to fail gracefully.  For example, with PCI hot plugging,
> you have rely on the fact that the PCI Base Address Registers of
> the newly inserted card have not yet been set, so it's IO is not
> yet initialized.  In the case of USB, you rely on the fact that
> the USB port is automatically put into a "device present but not
> connected" state by the USB hub.

You are absolutely correct. I just wish the device to remain in that state
until the running agent has terminated. I believe I have shown the race 
condition that arises otherwise.

> 	Perhaps your concern is about simple data corruption while
> reading /proc?  That is, perhaps you think the result of the read
> will return something that some combination of recent states of
> each slot on the bus (e.g., perhaps you read half of a device ID
> when device A is plugged in and half of the ID when device B is
> plugged in)?  That can be addressed by just having any file

I am worried about the content of that read being no longer true when you act 
upon it.

>
> Physical			/sbin/hotplug		/sbin/hotplug
> reality     Kernel		instance #1		instance #2
> insert A
> 	    interrupt?
> 	    new[dev]=1
>             /sbin/hotplug(dev)#1
> 				lock(dev)
> 				new[dev] = 0;
> 				insert_event(dev);
> 				[identifies device A]
> remove A
> 	    interrupt?
> 	    /sbin/hotplug(dev)#2
> insert B
> 	    interrupt?
> 	    new[dev] = 1;
> 	    /sbin/hotplug(dev)#3
> 							lock(dev)[blocks]
> 				devA->insert() fails

How do you guarantee that devA->insert() fails ?
I see no way.

> 				release_lock(dev)
> 				.			[lock(dev) returns]
> 				.			remove_event(dev);
> 				exit [whenever]		new[dev] = 0;
> 							remove_event(dev);
> 							[No remove handler
> 							registered since
> 							devA->insert failed.]
> 							insert_event(dev)
> 							[identifies device B]
> 							devB->insert()
> 							devB->insert succeeds
> 							release_lock(dev)
> 							exit
>

> >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
>
> 	^^^^^ this initialization will fail out.  In the meantime,

How ? Why ? That is an unfounded assumption. You cannot know that
an arbitrary initialisation will fail.

> under my scheme, the kernel has reset new[node0] = 1 and invoked
> a new /sbin/hotplug, which will run after the previous /sbin/hotplug
> releases its lock (say, an flock on a file or an SysV-IPC semaphore),
> and at that point will load the driver for device B.
>
> >release lock
> >remove for dev A - node 0
>
> [...]
>
> 	Let's look at some real hot plug busses for example.
>
> 	If USB kernel driver A is loaded while A is still plugged in, it
> will successfully identify A, and then the will get recognizable errors
> when it does its IO requests after device A is removed.
>
> 	If A was removed before driver A loads (and, in your example
> device B is inserted), the device desciptors and interface descriptors will
> not match what the driver wants, and initialization will return failure.

How do you guarantee that the user space agent doesn't do something
that doesn't involve the device directly ?
It could start a demon, change permissions on device nodes, change 
/etc/fstab, ...
In fact if it were used only to load a device driver, the kernel could just 
call kmod.

> 	Under CardBus (PCI), the a similar thing will happen using PCI
> device and vendor ID's.  The newly inserted device B's PCI base address
> registers will not be set at that point, so driver A does not have
> to worry about poking IO ports on the newly inserted card B (the code
> that actually reads the device ID's and sets that BAR's does have to
> be careful and watch the appropriate flags in case this occurs).

If you take a look at the USB code, you'll see that the call of the user 
space agent comes _after_ the driver has been probed for.
Thus the driver is bound to the device and functional.

Yet the user space agent may very well wish to access the device, thus there 
is no alternative.

> 	I can assume that deviceA->remove() can undo deviceA->insert() if
> deviceA->insert() ran to completion and returned "success."

Provided it was run on a device it was intended for.
Otherwise you depend on the specifics of the insert method,
which IMHO kernel code must not do.

> 	When init_typeA can falsely succeed on deviceB, remove_typeA
> will successfully shut it down a moment later and init_typeB() will

How do you guarantee that ? I can see no reason you could simply assume 
anything about the result of an initialisation done on a device of type B 
intended for a device of type A.

> then be run and initialize device B correctly.  The truely brief period
> when deviceB is misinitialized (the new /sbin/hotplug is just waiting
> for the semaphore from the previous one to be released) occurs under
> my scheme in a *subset* of the times when it occurs under yours.

Why ? Please explain. Obviously I consider my scheme invulnerable in that 
regard. ;-)

> Also, the only reason why this can happen is when this situation
> is considered harmless enough so that init_typeA() does not have code
> to guard against it (it can always scrutinize the device ID information
> more carefully and check that the equivalent of new[dev] is not set to
> guard against changes after it has read the device ID information).

What if the check of new[] happens before the device is exchanged ?
Is there any way you can make
if (check_identity_really_well(dev))
	intialise(dev);

safe without a lock ?

Now suppose it doesn't need iwconfig (reasonable some don't) and the user 
doesn't want dhcp on a wireless card.

Now you have (if you are really unlucky):

Physical reality				Computer
insert regular ethernet (dev A)
					/sbin/hotplug #1 starts
					sees regular ethernet (dev A)
remove regular ethernet (dev A)
insert wireless ethernet (dev B)
					dhclient eth0 &
					/sbin/hotplug exits

Isn't that an error that cannot be prevented by this design ?					

Now to the timeline for lost removal:

------------------------------------
First device plugged in
------------------------------------
         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])) {
                         was_plugged_in[dev] = 0;
                         handle_remove_event(dev);
                 }
                 if (new[dev]) {
----------------------------------
device removal: new[dev] =1
----------------------------------
                         new[dev] = 0;
                         if (is_plugged_in[dev]) {
                                 was_plugged_in[dev] = 1;
                                 handle_insert_event(device);
                         }
                 }
                 release_lock(dev);
         }

-------------------------------------
second instance of script getting lock, still new[] = 0
-------------------------------------
                 if (was_plugged_in[dev]
                    && (new[dev] ||
-------------------------------------
device addition: new[] = 1
------------------------------------- 
		!is_plugged_in[dev])) {
-------------------------------------
test has failed -> removal lost
-------------------------------------
                         was_plugged_in[dev] = 0;
                         handle_remove_event(dev);
                 }

I hope you can understand it this way, I don't have the real estate
on screen for the conventional way of showing this.

	Kindest Regards (I hope I have made myself clear now)
		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