[RFC patch 1/3] ieee1394: raw1394: replace BKL by local mutex, make ioctl() and mmap() thread-safe

[RFC patch 1/3] ieee1394: raw1394: replace BKL by local mutex, make ioctl() and mmap() thread-safe

[Stefan Richter]

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 <stefanr@s5r6.in-berlin.de>
---
 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 <linux/fs.h>
 #include <linux/poll.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/vmalloc.h>
@@ -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);

-- 
Stefan Richter
-=====-==--- =--- =----
http://arcgraph.de/sr/

[RFC patch 2/3] ieee1394: raw1394: narrow down the state_mutex protected region

[Stefan Richter]

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>
---
 drivers/ieee1394/raw1394.c |  207 +++++++++++++++++++------------------
 1 file changed, 108 insertions(+), 99 deletions(-)

Index: linux/drivers/ieee1394/raw1394.c
===================================================================
--- linux.orig/drivers/ieee1394/raw1394.c
+++ linux/drivers/ieee1394/raw1394.c
@@ -2556,102 +2556,106 @@ static int raw1394_mmap(struct file *fil
 	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
 		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;
 }
 
@@ -2734,7 +2745,6 @@ static long raw1394_compat_ioctl(struct 
 	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:
@@ -2767,7 +2777,6 @@ static long raw1394_compat_ioctl(struct 
 		err = -EINVAL;
 		break;
 	}
-	mutex_unlock(&fi->state_mutex);
 
 	return err;
 }

-- 
Stefan Richter
-=====-==--- =--- =----
http://arcgraph.de/sr/

[RFC patch 3/3] ieee1394: raw1394: make write() thread-safe

[Stefan Richter]

Application programs should use a libraw1394 handle only in a single
thread.  The raw1394 driver was apparently relying on this, because it
did nothing to protect its fi->state variable from corruption due to
concurrent accesses.

We now serialize the fi->state accesses.  This affects the write() path.
We re-use the state_mutex which was introduced to protect fi->iso_state
accesses in the ioctl() path.  These paths and accesses are independent
of each other, hence separate mutexes could be used.  But I don't see
much benefit in that.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/raw1394.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux/drivers/ieee1394/raw1394.c
===================================================================
--- linux.orig/drivers/ieee1394/raw1394.c
+++ linux/drivers/ieee1394/raw1394.c
@@ -2268,6 +2268,8 @@ static ssize_t raw1394_write(struct file
 		return -EFAULT;
 	}
 
+	mutex_lock(&fi->state_mutex);
+
 	switch (fi->state) {
 	case opened:
 		retval = state_opened(fi, req);
@@ -2282,6 +2284,8 @@ static ssize_t raw1394_write(struct file
 		break;
 	}
 
+	mutex_unlock(&fi->state_mutex);
+
 	if (retval < 0) {
 		free_pending_request(req);
 	} else {

-- 
Stefan Richter
-=====-==--- =--- =----
http://arcgraph.de/sr/

Re: [RFC patch 2/3] ieee1394: raw1394: narrow down the state_mutex protected region

[Stefan Richter]

I wrote:
> -static long do_raw1394_ioctl(struct file *file, unsigned int cmd,
> -							unsigned long arg)

This compiles a lot better with the following two hunks added:

@@ -2710,7 +2721,7 @@ static long raw1394_iso_xmit_recv_packet
 	    !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;
 }
@@ -2751,7 +2761,7 @@ static long raw1394_compat_ioctl(struct 
 	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:


-------- 8< --------

From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: 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>
---
 drivers/ieee1394/raw1394.c |  211 +++++++++++++++++++------------------
 1 file changed, 110 insertions(+), 101 deletions(-)

Index: linux-2.6.27-rc3/drivers/ieee1394/raw1394.c
===================================================================
--- linux-2.6.27-rc3.orig/drivers/ieee1394/raw1394.c
+++ linux-2.6.27-rc3/drivers/ieee1394/raw1394.c
@@ -2556,102 +2556,106 @@ static int raw1394_mmap(struct file *fil
 	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
 		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_packet
 	    !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 
 	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 
 	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 
 		err = -EINVAL;
 		break;
 	}
-	mutex_unlock(&fi->state_mutex);
 
 	return err;
 }

-- 
Stefan Richter
-=====-==--- =--- =----
http://arcgraph.de/sr/