[PATCH 1/5] check for proper registration with device core

[Dominik Brodowski]

On Tue, May 11, 2004 at 10:49:05PM +0100, Russell King wrote:
> On Tue, May 11, 2004 at 09:28:29PM +0200, Dominik Brodowski wrote:
> > Fail if registration of socket with driver core failed. This is necessary
> > so that socket-related sysfs entries will appear always if a socket thread
> > is up and running.
> 
> Ok, if we're going to make this change, I want to hear _success_ reports
> for each of the following cases (and I shall be ticking them off):
> 
> - non-modular, pcmcia card in socket during boot
success
> - non-modular, pcmcia card inserted after boot
success
> - non-modular, cardbus card in socket during boot
success
> - non-modular, cardbus card inserted after boot
success
> - modular, pcmcia card in socket during boot/module insertion
success
> - modular, pcmcia card inserted after boot/module insertion
success
> - modular, cardbus card in socket during boot/module insertion
success
> - modular, cardbus card inserted after boot/module insertion
success


> In the past, we've had _lots_ of problems with this and I want to make
> reasonably sure that we don't re-introduce either the PCMCIA vs SYSFS
> deadlocks or the PCMCIA card initialisation race condition.

OK, let's analyze why it can't possibly pose a problem eve

pcmcia_register_socket() is called by individual pcmcia socket drivers,
which may hold xxx_bus_type semaphores as these calls are usually done
during ->probe() callbacks. So, we may not attempt to down any _bus_type
semaphore again until wait_for_completion(&socket->thread_done) is reached.
On the other hand, this thread does _not_ hold the FIXME FIXME

pccardd() registers a class_device of type pcmcia_socket. Note that
it does not yet down the skt->skt_sem! That's only done later, in the
events if-clause. During class_device_register, it takes the 
pcmcia_socket_class's semaphore, and while holding it, it might call ds.c's 
pcmcia_bus_add_socket(). That one calls pcmcia_register_client(), which 
downs skt->skt_sem() [which neither the "insmod" thread nor this thread
holds so far]. Now we get to
    if ((s->state & (SOCKET_PRESENT|SOCKET_CARDBUS)) == SOCKET_PRESENT) {
	if (client->EventMask & CS_EVENT_CARD_INSERTION)
	    EVENT(client, CS_EVENT_CARD_INSERTION, CS_EVENT_PRI_LOW);
	else
	    client->PendingEvents |= CS_EVENT_CARD_INSERTION;
    }
So if it's a cardbus card, we're out of pcmcia_register_client(), out of
pcmcia_bus_add_socket(), thread_done is completed, and all threads are
happily running away.
What happens if it's a pcmcia card: then ds_event() is called. Currently,
ds_event() only informs userspace cardmgr[*] of the event by queueing it,
then it returns, pcmcia_register_client() returns, so does
pcmcia_bus_add_socket(), and again thread_done is completed without
problems. The cardmgr thread can then attempt to take any lock, there'll be
no dead-lock.
What happens if pcmcia device registration and/or binding is done in the 
kernel? It surely will try to grab the pcmcia_bus semaphore, but that's
different to the pcmcia_socket_class semaphore. It may grab other _class
sockets in respective pcmcia device drivers ->attach() or ->probe()
functions -- but to my knowledge no PCMCIA card supports having devices
[i.e. another bus!] below itself, so that there can be no contention to
the original socket's _bus_type semaphore.


However: it might get a bit easier if the class_device_register() call is
moved a bit higher, and pcmcia_register_client() won't call ds_event as it
doesn't know of a card being present _yet_. How about (untested):

static int pccardd(void *__skt)
{
	struct pcmcia_socket *skt = __skt;
	DECLARE_WAITQUEUE(wait, current);
	int ret;

	daemonize("pccardd");

	skt->socket = dead_socket;
	skt->state = 0;

	/* register with the device core */
	ret = class_device_register(&skt->dev);
	if (ret || pccard_sysfs_init(skt)) {
		printk(KERN_WARNING "PCMCIA: unable to register socket 0x%p\n", skt);
		skt->thread = NULL;
		complete_and_exit(&skt->thread_done, 0);
	}

	skt->thread = current;
	skt->ops->init(skt);
	skt->ops->set_socket(skt, &skt->socket);
	complete(&skt->thread_done);




	Dominik


[*] if it gets the chance to: cardmgr only handles sockets it knows of
during its start-up process, so that one needs to have run exactly in the
meantime. Anyway, let's continue this worst-case scenario.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.infradead.org/pipermail/linux-pcmcia/attachments/20040512/5d86777b/attachment.bin