[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