ieee1394: raw1394: narrow down the state_mutex protected region

Refactor the ioctl dispatcher in order to move a fraction of it out of
the section which is serialized by fi->state_mutex.  This is not so much
about performance but more about self-documentation:  The mutex_lock()/
mutex_unlock() calls are now closer to the data accesses which the mutex
protects, i.e. to the iso_state switch.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c
index 975ed70..d159442 100644 (file)
--- a/drivers/ieee1394/raw1394.c
+++ b/drivers/ieee1394/raw1394.c
@@ -2556,102 +2556,106 @@ static int raw1394_mmap(struct file *file, struct vm_area_struct *vma)
        return ret;
 }
 
-/* ioctl is only used for rawiso operations */
-static long do_raw1394_ioctl(struct file *file, unsigned int cmd,
-                                                       unsigned long arg)
+static long raw1394_ioctl_inactive(struct file_info *fi, unsigned int cmd,
+                                  void __user *argp)
+{
+       switch (cmd) {
+       case RAW1394_IOC_ISO_XMIT_INIT:
+               return raw1394_iso_xmit_init(fi, argp);
+       case RAW1394_IOC_ISO_RECV_INIT:
+               return raw1394_iso_recv_init(fi, argp);
+       default:
+               return -EINVAL;
+       }
+}
+
+static long raw1394_ioctl_recv(struct file_info *fi, unsigned int cmd,
+                              unsigned long arg)
 {
-       struct file_info *fi = file->private_data;
        void __user *argp = (void __user *)arg;
 
-       switch (fi->iso_state) {
-       case RAW1394_ISO_INACTIVE:
-               switch (cmd) {
-               case RAW1394_IOC_ISO_XMIT_INIT:
-                       return raw1394_iso_xmit_init(fi, argp);
-               case RAW1394_IOC_ISO_RECV_INIT:
-                       return raw1394_iso_recv_init(fi, argp);
-               default:
-                       break;
+       switch (cmd) {
+       case RAW1394_IOC_ISO_RECV_START:{
+                       int args[3];
+
+                       if (copy_from_user(&args[0], argp, sizeof(args)))
+                               return -EFAULT;
+                       return hpsb_iso_recv_start(fi->iso_handle,
+                                                  args[0], args[1], args[2]);
                }
-               break;
-       case RAW1394_ISO_RECV:
-               switch (cmd) {
-               case RAW1394_IOC_ISO_RECV_START:{
-                               /* copy args from user-space */
-                               int args[3];
-                               if (copy_from_user
-                                   (&args[0], argp, sizeof(args)))
-                                       return -EFAULT;
-                               return hpsb_iso_recv_start(fi->iso_handle,
-                                                          args[0], args[1],
-                                                          args[2]);
-                       }
-               case RAW1394_IOC_ISO_XMIT_RECV_STOP:
-                       hpsb_iso_stop(fi->iso_handle);
-                       return 0;
-               case RAW1394_IOC_ISO_RECV_LISTEN_CHANNEL:
-                       return hpsb_iso_recv_listen_channel(fi->iso_handle,
-                                                           arg);
-               case RAW1394_IOC_ISO_RECV_UNLISTEN_CHANNEL:
-                       return hpsb_iso_recv_unlisten_channel(fi->iso_handle,
-                                                             arg);
-               case RAW1394_IOC_ISO_RECV_SET_CHANNEL_MASK:{
-                               /* copy the u64 from user-space */
-                               u64 mask;
-                               if (copy_from_user(&mask, argp, sizeof(mask)))
-                                       return -EFAULT;
-                               return hpsb_iso_recv_set_channel_mask(fi->
-                                                                     iso_handle,
-                                                                     mask);
-                       }
-               case RAW1394_IOC_ISO_GET_STATUS:
-                       return raw1394_iso_get_status(fi, argp);
-               case RAW1394_IOC_ISO_RECV_PACKETS:
-                       return raw1394_iso_recv_packets(fi, argp);
-               case RAW1394_IOC_ISO_RECV_RELEASE_PACKETS:
-                       return hpsb_iso_recv_release_packets(fi->iso_handle,
-                                                            arg);
-               case RAW1394_IOC_ISO_RECV_FLUSH:
-                       return hpsb_iso_recv_flush(fi->iso_handle);
-               case RAW1394_IOC_ISO_SHUTDOWN:
-                       raw1394_iso_shutdown(fi);
-                       return 0;
-               case RAW1394_IOC_ISO_QUEUE_ACTIVITY:
-                       queue_rawiso_event(fi);
-                       return 0;
+       case RAW1394_IOC_ISO_XMIT_RECV_STOP:
+               hpsb_iso_stop(fi->iso_handle);
+               return 0;
+       case RAW1394_IOC_ISO_RECV_LISTEN_CHANNEL:
+               return hpsb_iso_recv_listen_channel(fi->iso_handle, arg);
+       case RAW1394_IOC_ISO_RECV_UNLISTEN_CHANNEL:
+               return hpsb_iso_recv_unlisten_channel(fi->iso_handle, arg);
+       case RAW1394_IOC_ISO_RECV_SET_CHANNEL_MASK:{
+                       u64 mask;
+
+                       if (copy_from_user(&mask, argp, sizeof(mask)))
+                               return -EFAULT;
+                       return hpsb_iso_recv_set_channel_mask(fi->iso_handle,
+                                                             mask);
                }
-               break;
-       case RAW1394_ISO_XMIT:
-               switch (cmd) {
-               case RAW1394_IOC_ISO_XMIT_START:{
-                               /* copy two ints from user-space */
-                               int args[2];
-                               if (copy_from_user
-                                   (&args[0], argp, sizeof(args)))
-                                       return -EFAULT;
-                               return hpsb_iso_xmit_start(fi->iso_handle,
-                                                          args[0], args[1]);
-                       }
-               case RAW1394_IOC_ISO_XMIT_SYNC:
-                       return hpsb_iso_xmit_sync(fi->iso_handle);
-               case RAW1394_IOC_ISO_XMIT_RECV_STOP:
-                       hpsb_iso_stop(fi->iso_handle);
-                       return 0;
-               case RAW1394_IOC_ISO_GET_STATUS:
-                       return raw1394_iso_get_status(fi, argp);
-               case RAW1394_IOC_ISO_XMIT_PACKETS:
-                       return raw1394_iso_send_packets(fi, argp);
-               case RAW1394_IOC_ISO_SHUTDOWN:
-                       raw1394_iso_shutdown(fi);
-                       return 0;
-               case RAW1394_IOC_ISO_QUEUE_ACTIVITY:
-                       queue_rawiso_event(fi);
-                       return 0;
+       case RAW1394_IOC_ISO_GET_STATUS:
+               return raw1394_iso_get_status(fi, argp);
+       case RAW1394_IOC_ISO_RECV_PACKETS:
+               return raw1394_iso_recv_packets(fi, argp);
+       case RAW1394_IOC_ISO_RECV_RELEASE_PACKETS:
+               return hpsb_iso_recv_release_packets(fi->iso_handle, arg);
+       case RAW1394_IOC_ISO_RECV_FLUSH:
+               return hpsb_iso_recv_flush(fi->iso_handle);
+       case RAW1394_IOC_ISO_SHUTDOWN:
+               raw1394_iso_shutdown(fi);
+               return 0;
+       case RAW1394_IOC_ISO_QUEUE_ACTIVITY:
+               queue_rawiso_event(fi);
+               return 0;
+       default:
+               return -EINVAL;
+       }
+}
+
+static long raw1394_ioctl_xmit(struct file_info *fi, unsigned int cmd,
+                              void __user *argp)
+{
+       switch (cmd) {
+       case RAW1394_IOC_ISO_XMIT_START:{
+                       int args[2];
+
+                       if (copy_from_user(&args[0], argp, sizeof(args)))
+                               return -EFAULT;
+                       return hpsb_iso_xmit_start(fi->iso_handle,
+                                                  args[0], args[1]);
                }
-               break;
+       case RAW1394_IOC_ISO_XMIT_SYNC:
+               return hpsb_iso_xmit_sync(fi->iso_handle);
+       case RAW1394_IOC_ISO_XMIT_RECV_STOP:
+               hpsb_iso_stop(fi->iso_handle);
+               return 0;
+       case RAW1394_IOC_ISO_GET_STATUS:
+               return raw1394_iso_get_status(fi, argp);
+       case RAW1394_IOC_ISO_XMIT_PACKETS:
+               return raw1394_iso_send_packets(fi, argp);
+       case RAW1394_IOC_ISO_SHUTDOWN:
+               raw1394_iso_shutdown(fi);
+               return 0;
+       case RAW1394_IOC_ISO_QUEUE_ACTIVITY:
+               queue_rawiso_event(fi);
+               return 0;
        default:
-               break;
+               return -EINVAL;
        }
+}
+
+/* ioctl is only used for rawiso operations */
+static long raw1394_ioctl(struct file *file, unsigned int cmd,
+                         unsigned long arg)
+{
+       struct file_info *fi = file->private_data;
+       void __user *argp = (void __user *)arg;
+       long ret;
 
        /* state-independent commands */
        switch(cmd) {
@@ -2661,18 +2665,25 @@ static long do_raw1394_ioctl(struct file *file, unsigned int cmd,
                break;
        }
 
-       return -EINVAL;
-}
+       mutex_lock(&fi->state_mutex);
 
-static long raw1394_ioctl(struct file *file, unsigned int cmd,
-                                                       unsigned long arg)
-{
-       struct file_info *fi = file->private_data;
-       long ret;
+       switch (fi->iso_state) {
+       case RAW1394_ISO_INACTIVE:
+               ret = raw1394_ioctl_inactive(fi, cmd, argp);
+               break;
+       case RAW1394_ISO_RECV:
+               ret = raw1394_ioctl_recv(fi, cmd, arg);
+               break;
+       case RAW1394_ISO_XMIT:
+               ret = raw1394_ioctl_xmit(fi, cmd, argp);
+               break;
+       default:
+               ret = -EINVAL;
+               break;
+       }
 
-       mutex_lock(&fi->state_mutex);
-       ret = do_raw1394_ioctl(file, cmd, arg);
        mutex_unlock(&fi->state_mutex);
+
        return ret;
 }
 
@@ -2710,7 +2721,7 @@ static long raw1394_iso_xmit_recv_packets32(struct file *file, unsigned int cmd,
            !copy_from_user(&infos32, &arg->infos, sizeof infos32)) {
                infos = compat_ptr(infos32);
                if (!copy_to_user(&dst->infos, &infos, sizeof infos))
-                       err = do_raw1394_ioctl(file, cmd, (unsigned long)dst);
+                       err = raw1394_ioctl(file, cmd, (unsigned long)dst);
        }
        return err;
 }
@@ -2734,7 +2745,6 @@ static long raw1394_compat_ioctl(struct file *file,
        void __user *argp = (void __user *)arg;
        long err;
 
-       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:
@@ -2751,7 +2761,7 @@ static long raw1394_compat_ioctl(struct file *file,
        case RAW1394_IOC_ISO_GET_STATUS:
        case RAW1394_IOC_ISO_SHUTDOWN:
        case RAW1394_IOC_ISO_QUEUE_ACTIVITY:
-               err = do_raw1394_ioctl(file, cmd, arg);
+               err = raw1394_ioctl(file, cmd, arg);
                break;
        /* These request have different format. */
        case RAW1394_IOC_ISO_RECV_PACKETS32:
@@ -2767,7 +2777,6 @@ static long raw1394_compat_ioctl(struct file *file,
                err = -EINVAL;
                break;
        }
-       mutex_unlock(&fi->state_mutex);
 
        return err;
 }