[PATCH 2.6] PCMCIA updates

Russell King rmk+pcmcia at arm.linux.org.uk
Sun Jan 18 19:57:39 GMT 2004


I'm going to be sending the changes below to Linus/Andrew within the
next day or so - this is the start of me unblocking the queue of PCMCIA
changes.  Some of this patch has been seen before, but other bits
haven't. The individual change comments are listed below.  I've tested
this stuff on both ARM and x86 platforms, and it appears fine for me.

[PCMCIA] Remove unused variable warnings.
  
  Remove unused variable 'i' in fops methods.  Fix debug macros which
  were the sole consumers of this variable.

[PCMCIA] Remove write-only socket_dev
  
  No need for a local pointer for the struct device, especially when
  it is only ever written.  If necessary, the device can be accessed
  using s->parent->dev.dev

[PCMCIA] Get rid of racy interruptible_sleep_on()
  
  ds.c uses interruptible_sleep_on() without any protection.  Use
  wait_event_interruptible() instead.
  
  In addition, fix a bug where threads waiting for cardmgr events to
  complete were left waiting if cardmgr exited.

[PCMCIA] Add refcounting to struct pcmcia_bus_socket
  
  If you perform the following commands in order:
  
   # cardctl eject
   # rmmod yenta_socket
   # insmod drivers/pcmcia/yenta_socket.ko
   # killall cardmgr
  
  the rmmod ends up freeing the pcmcia_bus_socket while the wait
  queue is still active.  The killall cardmgr cases the the select()
  to complete, and users to be removed from the "queue" - which ends
  up writing to freed memory.
  
  The following patch adds refcounting to pcmcia_bus_socket so we
  won't free it until all users have gone.  We also add "SOCKET_DEAD"
  to mark the condition where the socket is no longer present in the
  system.
  
  Note that we don't wake up cardmgr when we remove sockets -
  unfortunately cardmgr doesn't like receiving errors from read().
  Really, cardmgr should treat EIO from read() as a fatal error
  for that socket, and stop listening for events from it.

diff -Nru a/drivers/pcmcia/ds.c b/drivers/pcmcia/ds.c
--- a/drivers/pcmcia/ds.c	Sun Jan 18 19:50:51 2004
+++ b/drivers/pcmcia/ds.c	Sun Jan 18 19:50:51 2004
@@ -51,6 +51,8 @@
 #include <linux/list.h>
 #include <linux/workqueue.h>
 
+#include <asm/atomic.h>
+
 #include <pcmcia/version.h>
 #include <pcmcia/cs_types.h>
 #include <pcmcia/cs.h>
@@ -95,10 +97,12 @@
     int			event_head, event_tail;
     event_t		event[MAX_EVENTS];
     struct user_info_t	*next;
+    struct pcmcia_bus_socket *socket;
 } user_info_t;
 
 /* Socket state information */
 struct pcmcia_bus_socket {
+	atomic_t		refcount;
 	client_handle_t		handle;
 	int			state;
 	user_info_t		*user;
@@ -106,13 +110,13 @@
 	wait_queue_head_t	queue, request;
 	struct work_struct	removal;
 	socket_bind_t		*bind;
-	struct device		*socket_dev;
 	struct pcmcia_socket	*parent;
 };
 
 #define SOCKET_PRESENT		0x01
 #define SOCKET_BUSY		0x02
 #define SOCKET_REMOVAL_PENDING	0x10
+#define SOCKET_DEAD		0x80
 
 /*====================================================================*/
 
@@ -137,6 +141,24 @@
 static struct pcmcia_driver * get_pcmcia_driver (dev_info_t *dev_info);
 static struct pcmcia_bus_socket * get_socket_info_by_nr(unsigned int nr);
 
+static void pcmcia_put_bus_socket(struct pcmcia_bus_socket *s)
+{
+	if (atomic_dec_and_test(&s->refcount))
+		kfree(s);
+}
+
+static struct pcmcia_bus_socket *pcmcia_get_bus_socket(int nr)
+{
+	struct pcmcia_bus_socket *s;
+
+	s = get_socket_info_by_nr(nr);
+	if (s) {
+		WARN_ON(atomic_read(&s->refcount) == 0);
+		atomic_inc(&s->refcount);
+	}
+	return s;
+}
+
 /**
  * pcmcia_register_driver - register a PCMCIA driver with the bus core
  *
@@ -230,13 +252,10 @@
     if (s->state & SOCKET_BUSY)
 	s->req_pending = 1;
     handle_event(s, event);
-    if (s->req_pending > 0) {
-	interruptible_sleep_on(&s->request);
-	if (signal_pending(current))
-	    return CS_IN_USE;
-	else
-	    return s->req_result;
-    }
+    if (wait_event_interruptible(s->request, s->req_pending <= 0))
+        return CS_IN_USE;
+    if (s->state & SOCKET_BUSY)
+        return s->req_result;
     return CS_SUCCESS;
 }
 
@@ -501,7 +520,7 @@
 
     DEBUG(0, "ds_open(socket %d)\n", i);
 
-    s = get_socket_info_by_nr(i);
+    s = pcmcia_get_bus_socket(i);
     if (!s)
 	    return -ENODEV;
 
@@ -517,6 +536,7 @@
     user->event_tail = user->event_head = 0;
     user->next = s->user;
     user->user_magic = USER_MAGIC;
+    user->socket = s;
     s->user = user;
     file->private_data = user;
     
@@ -529,23 +549,23 @@
 
 static int ds_release(struct inode *inode, struct file *file)
 {
-    socket_t i = iminor(inode);
     struct pcmcia_bus_socket *s;
     user_info_t *user, **link;
 
-    DEBUG(0, "ds_release(socket %d)\n", i);
-
-    s = get_socket_info_by_nr(i);
-    if (!s)
-	    return 0;
+    DEBUG(0, "ds_release(socket %d)\n", iminor(inode));
 
     user = file->private_data;
     if (CHECK_USER(user))
 	goto out;
 
+    s = user->socket;
+
     /* Unlink user data structure */
-    if ((file->f_flags & O_ACCMODE) != O_RDONLY)
+    if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
 	s->state &= ~SOCKET_BUSY;
+	s->req_pending = 0;
+	wake_up_interruptible(&s->request);
+    }
     file->private_data = NULL;
     for (link = &s->user; *link; link = &(*link)->next)
 	if (*link == user) break;
@@ -554,6 +574,7 @@
     *link = user->next;
     user->user_magic = 0;
     kfree(user);
+    pcmcia_put_bus_socket(s);
 out:
     return 0;
 } /* ds_release */
@@ -563,30 +584,28 @@
 static ssize_t ds_read(struct file *file, char *buf,
 		       size_t count, loff_t *ppos)
 {
-    socket_t i = iminor(file->f_dentry->d_inode);
     struct pcmcia_bus_socket *s;
     user_info_t *user;
+    int ret;
 
-    DEBUG(2, "ds_read(socket %d)\n", i);
+    DEBUG(2, "ds_read(socket %d)\n", iminor(inode));
     
     if (count < 4)
 	return -EINVAL;
 
-    s = get_socket_info_by_nr(i);
-    if (!s)
-	    return -ENODEV;
-
     user = file->private_data;
     if (CHECK_USER(user))
 	return -EIO;
     
-    if (queue_empty(user)) {
-	interruptible_sleep_on(&s->queue);
-	if (signal_pending(current))
-	    return -EINTR;
-    }
+    s = user->socket;
+    if (s->state & SOCKET_DEAD)
+        return -EIO;
+
+    ret = wait_event_interruptible(s->queue, !queue_empty(user));
+    if (ret == 0)
+	ret = put_user(get_queued_event(user), (int *)buf) ? -EFAULT : 4;
 
-    return put_user(get_queued_event(user), (int *)buf) ? -EFAULT : 4;
+    return ret;
 } /* ds_read */
 
 /*====================================================================*/
@@ -594,25 +613,24 @@
 static ssize_t ds_write(struct file *file, const char *buf,
 			size_t count, loff_t *ppos)
 {
-    socket_t i = iminor(file->f_dentry->d_inode);
     struct pcmcia_bus_socket *s;
     user_info_t *user;
 
-    DEBUG(2, "ds_write(socket %d)\n", i);
+    DEBUG(2, "ds_write(socket %d)\n", iminor(inode));
     
     if (count != 4)
 	return -EINVAL;
     if ((file->f_flags & O_ACCMODE) == O_RDONLY)
 	return -EBADF;
 
-    s = get_socket_info_by_nr(i);
-    if (!s)
-	    return -ENODEV;
-
     user = file->private_data;
     if (CHECK_USER(user))
 	return -EIO;
 
+    s = user->socket;
+    if (s->state & SOCKET_DEAD)
+        return -EIO;
+
     if (s->req_pending) {
 	s->req_pending--;
 	get_user(s->req_result, (int *)buf);
@@ -629,19 +647,19 @@
 /* No kernel lock - fine */
 static u_int ds_poll(struct file *file, poll_table *wait)
 {
-    socket_t i = iminor(file->f_dentry->d_inode);
     struct pcmcia_bus_socket *s;
     user_info_t *user;
 
-    DEBUG(2, "ds_poll(socket %d)\n", i);
+    DEBUG(2, "ds_poll(socket %d)\n", iminor(inode));
     
-    s = get_socket_info_by_nr(i);
-    if (!s)
-	    return POLLERR;
-
     user = file->private_data;
     if (CHECK_USER(user))
 	return POLLERR;
+    s = user->socket;
+    /*
+     * We don't check for a dead socket here since that
+     * will send cardmgr into an endless spin.
+     */
     poll_wait(file, &s->queue, wait);
     if (!queue_empty(user))
 	return POLLIN | POLLRDNORM;
@@ -653,17 +671,21 @@
 static int ds_ioctl(struct inode * inode, struct file * file,
 		    u_int cmd, u_long arg)
 {
-    socket_t i = iminor(inode);
     struct pcmcia_bus_socket *s;
     u_int size;
     int ret, err;
     ds_ioctl_arg_t buf;
+    user_info_t *user;
 
-    DEBUG(2, "ds_ioctl(socket %d, %#x, %#lx)\n", i, cmd, arg);
+    DEBUG(2, "ds_ioctl(socket %d, %#x, %#lx)\n", iminor(inode), cmd, arg);
     
-    s = get_socket_info_by_nr(i);
-    if (!s)
-	    return -ENODEV;
+    user = file->private_data;
+    if (CHECK_USER(user))
+	return -EIO;
+
+    s = user->socket;
+    if (s->state & SOCKET_DEAD)
+        return -EIO;
     
     size = (cmd & IOCSIZE_MASK) >> IOCSIZE_SHIFT;
     if (size > sizeof(ds_ioctl_arg_t)) return -EINVAL;
@@ -833,6 +855,7 @@
 	if(!s)
 		return -ENOMEM;
 	memset(s, 0, sizeof(struct pcmcia_bus_socket));
+	atomic_set(&s->refcount, 1);
     
 	/*
 	 * Ugly. But we want to wait for the socket threads to have started up.
@@ -845,7 +868,6 @@
 	init_waitqueue_head(&s->request);
 
 	/* initialize data */
-	s->socket_dev = socket->dev.dev;
 	INIT_WORK(&s->removal, handle_removal, s);
 	s->parent = socket;
 
@@ -894,7 +916,8 @@
 
 	pcmcia_deregister_client(socket->pcmcia->handle);
 
-	kfree(socket->pcmcia);
+	socket->pcmcia->state |= SOCKET_DEAD;
+	pcmcia_put_bus_socket(socket->pcmcia);
 	socket->pcmcia = NULL;
 
 	return;


-- 
Russell King
 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



More information about the linux-pcmcia mailing list