Re: [PCMCIA] Xircom nic hang on boot since cs.c race condition patch

[Russell King]

On Wed, Sep 17, 2003 at 02:44:06PM -0400, Sean Estabrooks wrote:
> [PCMCIA] Fix race condition causing cards to be incorrectly recognised
> 
> This patch that went into test5 causes my Toshiba laptop with Xircom 
> pcmcia nic to freeze on boot at "Socket status: 30000020".  
> 
> test4 didn't have this issue, and test5-mm2 is the same as test5 vanilla.
> If the Xircom nic is inserted after boot then everything works without a 
> problem.
> 
> Backing out this patch restores normal boot.   Any tips on how i can 
> narrow the problem down further would be appreciated.   lspci and
> .config attached.

Ok, can you try the attached patch please?  It basically juggles the
initialisation so that we avoid the locking issues by moving the init
between our the socket driver and our private thread.

The patch is against Linus' tree as of last Wednesday.

Note that I haven't compile-tested this exact patch, (but one similar)
so I need feedback from both cardbus and pcmcia-using people before I
submit it.

===== drivers/pcmcia/cs.c 1.62 vs edited =====
--- 1.62/drivers/pcmcia/cs.c	Mon Sep  8 23:15:21 2003
+++ edited/drivers/pcmcia/cs.c	Sat Sep 20 22:13:27 2003
@@ -281,72 +281,29 @@
 EXPORT_SYMBOL(pcmcia_socket_dev_resume);
 
 
-static int pccardd(void *__skt);
-#define to_class_data(dev) dev->class_data
-
-static int pcmcia_add_socket(struct class_device *class_dev)
-{
-	struct pcmcia_socket *socket = class_get_devdata(class_dev);
-	int ret = 0;
-
-	/* base address = 0, map = 0 */
-	socket->cis_mem.flags = 0;
-	socket->cis_mem.speed = cis_speed;
-	socket->erase_busy.next = socket->erase_busy.prev = &socket->erase_busy;
-	INIT_LIST_HEAD(&socket->cis_cache);
-	spin_lock_init(&socket->lock);
-
-	init_completion(&socket->thread_done);
-	init_waitqueue_head(&socket->thread_wait);
-	init_MUTEX(&socket->skt_sem);
-	spin_lock_init(&socket->thread_lock);
-
-	socket->socket = dead_socket;
-	socket->ops->init(socket);
-
-	ret = kernel_thread(pccardd, socket, CLONE_KERNEL);
-	if (ret < 0)
-		return ret;
-
-	wait_for_completion(&socket->thread_done);
-	BUG_ON(!socket->thread);
-	pcmcia_parse_events(socket, SS_DETECT);
-
-	return 0;
-}
-
-static void pcmcia_remove_socket(struct class_device *class_dev)
+static void pcmcia_release_socket(struct class_device *class_dev)
 {
 	struct pcmcia_socket *socket = class_get_devdata(class_dev);
 	client_t *client;
 
-	if (socket->thread) {
-		init_completion(&socket->thread_done);
-		socket->thread = NULL;
-		wake_up(&socket->thread_wait);
-		wait_for_completion(&socket->thread_done);
-	}
-	release_cis_mem(socket);
 	while (socket->clients) {
 		client = socket->clients;
 		socket->clients = socket->clients->next;
 		kfree(client);
 	}
-	socket->ops = NULL;
-}
 
-static void pcmcia_release_socket(struct class_device *class_dev)
-{
-	struct pcmcia_socket *socket = class_get_devdata(class_dev);
 	complete(&socket->socket_released);
 }
 
+static int pccardd(void *__skt);
 
 /**
  * pcmcia_register_socket - add a new pcmcia socket device
  */
 int pcmcia_register_socket(struct pcmcia_socket *socket)
 {
+	int ret;
+
 	if (!socket || !socket->ops || !socket->dev.dev)
 		return -EINVAL;
 
@@ -381,15 +338,34 @@
 	socket->dev.class = &pcmcia_socket_class;
 	snprintf(socket->dev.class_id, BUS_ID_SIZE, "pcmcia_socket%u", socket->sock);
 
-	/* register with the device core */
-	if (class_device_register(&socket->dev)) {
-		down_write(&pcmcia_socket_list_rwsem);
-		list_del(&socket->socket_list);
-		up_write(&pcmcia_socket_list_rwsem);
-		return -EINVAL;
-	}
+	/* base address = 0, map = 0 */
+	socket->cis_mem.flags = 0;
+	socket->cis_mem.speed = cis_speed;
+	socket->erase_busy.next = socket->erase_busy.prev = &socket->erase_busy;
+	INIT_LIST_HEAD(&socket->cis_cache);
+	spin_lock_init(&socket->lock);
+
+	init_completion(&socket->socket_released);
+	init_completion(&socket->thread_done);
+	init_waitqueue_head(&socket->thread_wait);
+	init_MUTEX(&socket->skt_sem);
+	spin_lock_init(&socket->thread_lock);
+
+	ret = kernel_thread(pccardd, socket, CLONE_KERNEL);
+	if (ret < 0)
+		goto err;
+
+	wait_for_completion(&socket->thread_done);
+	BUG_ON(!socket->thread);
+	pcmcia_parse_events(socket, SS_DETECT);
 
 	return 0;
+
+ err:
+	down_write(&pcmcia_socket_list_rwsem);
+	list_del(&socket->socket_list);
+	up_write(&pcmcia_socket_list_rwsem);
+	return ret;
 } /* pcmcia_register_socket */
 EXPORT_SYMBOL(pcmcia_register_socket);
 
@@ -404,10 +380,13 @@
 
 	DEBUG(0, "cs: pcmcia_unregister_socket(0x%p)\n", socket->ops);
 
-	init_completion(&socket->socket_released);
-
-	/* remove from the device core */
-	class_device_unregister(&socket->dev);
+	if (socket->thread) {
+		init_completion(&socket->thread_done);
+		socket->thread = NULL;
+		wake_up(&socket->thread_wait);
+		wait_for_completion(&socket->thread_done);
+	}
+	release_cis_mem(socket);
 
 	/* remove from our own list */
 	down_write(&pcmcia_socket_list_rwsem);
@@ -783,11 +762,22 @@
 {
 	struct pcmcia_socket *skt = __skt;
 	DECLARE_WAITQUEUE(wait, current);
+	int ret;
 
 	daemonize("pccardd");
 	skt->thread = current;
 	complete(&skt->thread_done);
 
+	skt->socket = dead_socket;
+	skt->ops->init(skt);
+
+	/* register with the device core */
+	ret = class_device_register(&skt->dev);
+	if (ret) {
+		printk(KERN_WARNING "PCMCIA: unable to register socket 0x%p\n",
+			skt);
+	}
+
 	add_wait_queue(&skt->thread_wait, &wait);
 	for (;;) {
 		unsigned long flags;
@@ -823,6 +813,9 @@
 	}
 	remove_wait_queue(&skt->thread_wait, &wait);
 
+	/* remove from the device core */
+	class_device_unregister(&skt->dev);
+
 	complete_and_exit(&skt->thread_done, 0);
 }
 
@@ -2501,12 +2494,6 @@
 };
 EXPORT_SYMBOL(pcmcia_socket_class);
 
-static struct class_interface pcmcia_socket = {
-	.class = &pcmcia_socket_class,
-	.add = &pcmcia_add_socket,
-	.remove = &pcmcia_remove_socket,
-};
-
 
 static int __init init_pcmcia_cs(void)
 {
@@ -2514,7 +2501,6 @@
     printk(KERN_INFO "  %s\n", options);
     DEBUG(0, "%s\n", version);
     class_register(&pcmcia_socket_class);
-    class_interface_register(&pcmcia_socket);
 
     return 0;
 }
@@ -2523,7 +2509,6 @@
 {
     printk(KERN_INFO "unloading Kernel Card Services\n");
     release_resource_db();
-    class_interface_unregister(&pcmcia_socket);
     class_unregister(&pcmcia_socket_class);
 }
 


-- 
Russell King (rmk@xxxxxxxxxxxxxxxx)	http://www.arm.linux.org.uk/personal/
      Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
      maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                      2.6 Serial core
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/