Date: Thu, 22 Jul 2010 11:56:38 +0200 (CEST) From: Stefan Richter Subject: firewire: nosy: convert to unlocked ioctl The required serialization of NOSY_IOC_START and NOSY_IOC_STOP is already provided by the client_list_lock. NOSY_IOC_FILTER does not really require serialization since accesses to tcode_mask are atomic on any sane CPU architecture. Nevertheless, make it explicit that we want this to be atomic by means of client_list_lock (which also surrounds the other tcode_mask access in the IRQ handler). While we are at it, change the type of tcode_mask to u32 for consistency with the user API. NOSY_IOC_GET_STATS does not require serialization against itself. But there is a bug here regarding concurrent updates of the two counters by the IRQ handler. Fix it by taking the client_list_lock in this ioctl too. Signed-off-by: Stefan Richter --- drivers/firewire/nosy.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) Index: b/drivers/firewire/nosy.c =================================================================== --- a/drivers/firewire/nosy.c +++ b/drivers/firewire/nosy.c @@ -107,7 +107,7 @@ struct pcilynx { struct client { struct pcilynx *lynx; - unsigned long tcode_mask; + u32 tcode_mask; struct packet_buffer buffer; struct list_head link; }; @@ -350,17 +350,20 @@ nosy_read(struct file *file, char *buffe return packet_buffer_get(&client->buffer, buffer, count); } -static int -nosy_ioctl(struct inode *inode, struct file *file, - unsigned int cmd, unsigned long arg) +static long +nosy_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct client *client = file->private_data; + spinlock_t *client_list_lock = &client->lynx->client_list_lock; struct nosy_stats stats; switch (cmd) { case NOSY_IOC_GET_STATS: + spin_lock_irq(client_list_lock); stats.total_packet_count = client->buffer.total_packet_count; - stats.lost_packet_count = client->buffer.lost_packet_count; + stats.lost_packet_count = client->buffer.lost_packet_count; + spin_unlock_irq(client_list_lock); + if (copy_to_user((void *) arg, &stats, sizeof stats)) return -EFAULT; else @@ -375,7 +378,9 @@ nosy_ioctl(struct inode *inode, struct f return 0; case NOSY_IOC_FILTER: + spin_lock_irq(client_list_lock); client->tcode_mask = arg; + spin_unlock_irq(client_list_lock); return 0; default: @@ -385,12 +390,12 @@ nosy_ioctl(struct inode *inode, struct f } static const struct file_operations nosy_ops = { - .owner = THIS_MODULE, - .read = nosy_read, - .ioctl = nosy_ioctl, - .poll = nosy_poll, - .open = nosy_open, - .release = nosy_release, + .owner = THIS_MODULE, + .read = nosy_read, + .unlocked_ioctl = nosy_ioctl, + .poll = nosy_poll, + .open = nosy_open, + .release = nosy_release, }; #define PHY_PACKET_SIZE 12 /* 1 payload, 1 inverse, 1 ack = 3 quadlets */ @@ -408,7 +413,7 @@ packet_handler(struct pcilynx *lynx) { unsigned long flags; struct client *client; - unsigned long tcode_mask; + u32 tcode_mask; size_t length; struct link_packet *packet; struct timeval tv;