Date: Sat, 16 Aug 2008 00:11:48 +0200 (CEST) From: Stefan Richter Subject: ieee1394: raw1394: replace BKL by local mutex, make ioctl() and mmap() thread-safe This removes the last usage of the Big Kernel Lock from the ieee1394 stack, i.e. from raw1394's (unlocked_)ioctl and compat_ioctl. The ioctl()s don't need to take the BKL, but they need to be serialized per struct file *. In particular, accesses to ->iso_state need to be serial. We simply use a blocking mutex for this purpose because libraw1394 does not use O_NONBLOCK. In practice, there is no lock contention anyway because most if not all libraw1394 clients use a libraw1394 handle only in a single thread. mmap() also accesses ->iso_state. Until now this was unprotected against concurrent changes by ioctls. Fix this bug while we are at it. Signed-off-by: Stefan Richter --- drivers/ieee1394/raw1394-private.h | 1 + drivers/ieee1394/raw1394.c | 23 +++++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) Index: linux/drivers/ieee1394/raw1394-private.h =================================================================== --- linux.orig/drivers/ieee1394/raw1394-private.h +++ linux/drivers/ieee1394/raw1394-private.h @@ -22,6 +22,7 @@ enum raw1394_iso_state { RAW1394_ISO_INA struct file_info { struct list_head list; + struct mutex state_mutex; enum { opened, initialized, connected } state; unsigned int protocol_version; Index: linux/drivers/ieee1394/raw1394.c =================================================================== --- linux.orig/drivers/ieee1394/raw1394.c +++ linux/drivers/ieee1394/raw1394.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -2541,11 +2542,18 @@ static int raw1394_read_cycle_timer(stru static int raw1394_mmap(struct file *file, struct vm_area_struct *vma) { struct file_info *fi = file->private_data; + int ret; + + mutex_lock(&fi->state_mutex); if (fi->iso_state == RAW1394_ISO_INACTIVE) - return -EINVAL; + ret = -EINVAL; + else + ret = dma_region_mmap(&fi->iso_handle->data_buf, file, vma); + + mutex_unlock(&fi->state_mutex); - return dma_region_mmap(&fi->iso_handle->data_buf, file, vma); + return ret; } /* ioctl is only used for rawiso operations */ @@ -2659,10 +2667,12 @@ static long do_raw1394_ioctl(struct file static long raw1394_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { + struct file_info *fi = file->private_data; long ret; - lock_kernel(); + + mutex_lock(&fi->state_mutex); ret = do_raw1394_ioctl(file, cmd, arg); - unlock_kernel(); + mutex_unlock(&fi->state_mutex); return ret; } @@ -2724,7 +2734,7 @@ static long raw1394_compat_ioctl(struct void __user *argp = (void __user *)arg; long err; - lock_kernel(); + mutex_lock(&fi->state_mutex); switch (cmd) { /* These requests have same format as long as 'int' has same size. */ case RAW1394_IOC_ISO_RECV_INIT: @@ -2757,7 +2767,7 @@ static long raw1394_compat_ioctl(struct err = -EINVAL; break; } - unlock_kernel(); + mutex_unlock(&fi->state_mutex); return err; } @@ -2791,6 +2801,7 @@ static int raw1394_open(struct inode *in fi->notification = (u8) RAW1394_NOTIFY_ON; /* busreset notification */ INIT_LIST_HEAD(&fi->list); + mutex_init(&fi->state_mutex); fi->state = opened; INIT_LIST_HEAD(&fi->req_pending); INIT_LIST_HEAD(&fi->req_complete);