1 files changed, 80 insertions, 58 deletions

diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c
index 571ea68c0cf2..5ec4f5eb6b19 100644
--- a/drivers/ieee1394/raw1394.c
+++ b/drivers/ieee1394/raw1394.c
@@ -44,14 +44,15 @@
 #include <linux/compat.h>
 
 #include "csr1212.h"
+#include "highlevel.h"
+#include "hosts.h"
 #include "ieee1394.h"
-#include "ieee1394_types.h"
 #include "ieee1394_core.h"
-#include "nodemgr.h"
-#include "hosts.h"
-#include "highlevel.h"
-#include "iso.h"
+#include "ieee1394_hotplug.h"
 #include "ieee1394_transactions.h"
+#include "ieee1394_types.h"
+#include "iso.h"
+#include "nodemgr.h"
 #include "raw1394.h"
 #include "raw1394-private.h"
 
@@ -66,7 +67,7 @@
 #define DBGMSG(fmt, args...) \
 printk(KERN_INFO "raw1394:" fmt "\n" , ## args)
 #else
-#define DBGMSG(fmt, args...)
+#define DBGMSG(fmt, args...) do {} while (0)
 #endif
 
 static LIST_HEAD(host_info_list);
@@ -132,10 +133,9 @@ static void free_pending_request(struct pending_request *req)
 static void __queue_complete_req(struct pending_request *req)
 {
 	struct file_info *fi = req->file_info;
-	list_move_tail(&req->list, &fi->req_complete);
 
-	up(&fi->complete_sem);
-	wake_up_interruptible(&fi->poll_wait_complete);
+	list_move_tail(&req->list, &fi->req_complete);
+ 	wake_up(&fi->wait_complete);
 }
 
 static void queue_complete_req(struct pending_request *req)
@@ -463,13 +463,36 @@ raw1394_compat_read(const char __user *buf, struct raw1394_request *r)
 
 #endif
 
+/* get next completed request  (caller must hold fi->reqlists_lock) */
+static inline struct pending_request *__next_complete_req(struct file_info *fi)
+{
+	struct list_head *lh;
+	struct pending_request *req = NULL;
+
+	if (!list_empty(&fi->req_complete)) {
+		lh = fi->req_complete.next;
+		list_del(lh);
+		req = list_entry(lh, struct pending_request, list);
+	}
+	return req;
+}
+
+/* atomically get next completed request */
+static struct pending_request *next_complete_req(struct file_info *fi)
+{
+	unsigned long flags;
+	struct pending_request *req;
+
+	spin_lock_irqsave(&fi->reqlists_lock, flags);
+	req = __next_complete_req(fi);
+	spin_unlock_irqrestore(&fi->reqlists_lock, flags);
+	return req;
+}
 
 static ssize_t raw1394_read(struct file *file, char __user * buffer,
 			    size_t count, loff_t * offset_is_ignored)
 {
-	unsigned long flags;
 	struct file_info *fi = (struct file_info *)file->private_data;
-	struct list_head *lh;
 	struct pending_request *req;
 	ssize_t ret;
 
@@ -487,22 +510,21 @@ static ssize_t raw1394_read(struct file *file, char __user * buffer,
 	}
 
 	if (file->f_flags & O_NONBLOCK) {
-		if (down_trylock(&fi->complete_sem)) {
+		if (!(req = next_complete_req(fi)))
 			return -EAGAIN;
-		}
 	} else {
-		if (down_interruptible(&fi->complete_sem)) {
+		/*
+		 * NB: We call the macro wait_event_interruptible() with a
+		 * condition argument with side effect.  This is only possible
+		 * because the side effect does not occur until the condition
+		 * became true, and wait_event_interruptible() won't evaluate
+		 * the condition again after that.
+		 */
+		if (wait_event_interruptible(fi->wait_complete,
+					     (req = next_complete_req(fi))))
 			return -ERESTARTSYS;
-		}
 	}
 
-	spin_lock_irqsave(&fi->reqlists_lock, flags);
-	lh = fi->req_complete.next;
-	list_del(lh);
-	spin_unlock_irqrestore(&fi->reqlists_lock, flags);
-
-	req = list_entry(lh, struct pending_request, list);
-
 	if (req->req.length) {
 		if (copy_to_user(int2ptr(req->req.recvb), req->data,
 				 req->req.length)) {
@@ -1752,6 +1774,7 @@ static int arm_register(struct file_info *fi, struct pending_request *req)
 	addr->notification_options |= addr->client_transactions;
 	addr->recvb = req->req.recvb;
 	addr->rec_length = (u16) ((req->req.misc >> 16) & 0xFFFF);
+
 	spin_lock_irqsave(&host_info_lock, flags);
 	hi = find_host_info(fi->host);
 	same_host = 0;
@@ -1777,9 +1800,9 @@ static int arm_register(struct file_info *fi, struct pending_request *req)
 	}
 	if (same_host) {
 		/* addressrange occupied by same host */
+		spin_unlock_irqrestore(&host_info_lock, flags);
 		vfree(addr->addr_space_buffer);
 		kfree(addr);
-		spin_unlock_irqrestore(&host_info_lock, flags);
 		return (-EALREADY);
 	}
 	/* another host with valid address-entry containing same addressrange */
@@ -1807,6 +1830,8 @@ static int arm_register(struct file_info *fi, struct pending_request *req)
 			}
 		}
 	}
+	spin_unlock_irqrestore(&host_info_lock, flags);
+
 	if (another_host) {
 		DBGMSG("another hosts entry is valid -> SUCCESS");
 		if (copy_to_user(int2ptr(req->req.recvb),
@@ -1815,11 +1840,11 @@ static int arm_register(struct file_info *fi, struct pending_request *req)
 			       " address-range-entry is invalid -> EFAULT !!!\n");
 			vfree(addr->addr_space_buffer);
 			kfree(addr);
-			spin_unlock_irqrestore(&host_info_lock, flags);
 			return (-EFAULT);
 		}
 		free_pending_request(req);	/* immediate success or fail */
 		/* INSERT ENTRY */
+		spin_lock_irqsave(&host_info_lock, flags);
 		list_add_tail(&addr->addr_list, &fi->addr_list);
 		spin_unlock_irqrestore(&host_info_lock, flags);
 		return sizeof(struct raw1394_request);
@@ -1830,15 +1855,15 @@ static int arm_register(struct file_info *fi, struct pending_request *req)
 				    req->req.address + req->req.length);
 	if (retval) {
 		/* INSERT ENTRY */
+		spin_lock_irqsave(&host_info_lock, flags);
 		list_add_tail(&addr->addr_list, &fi->addr_list);
+		spin_unlock_irqrestore(&host_info_lock, flags);
 	} else {
 		DBGMSG("arm_register failed errno: %d \n", retval);
 		vfree(addr->addr_space_buffer);
 		kfree(addr);
-		spin_unlock_irqrestore(&host_info_lock, flags);
 		return (-EALREADY);
 	}
-	spin_unlock_irqrestore(&host_info_lock, flags);
 	free_pending_request(req);	/* immediate success or fail */
 	return sizeof(struct raw1394_request);
 }
@@ -1904,10 +1929,10 @@ static int arm_unregister(struct file_info *fi, struct pending_request *req)
 	if (another_host) {
 		DBGMSG("delete entry from list -> success");
 		list_del(&addr->addr_list);
+		spin_unlock_irqrestore(&host_info_lock, flags);
 		vfree(addr->addr_space_buffer);
 		kfree(addr);
 		free_pending_request(req);	/* immediate success or fail */
-		spin_unlock_irqrestore(&host_info_lock, flags);
 		return sizeof(struct raw1394_request);
 	}
 	retval =
@@ -1949,23 +1974,19 @@ static int arm_get_buf(struct file_info *fi, struct pending_request *req)
 		    (arm_addr->end > req->req.address)) {
 			if (req->req.address + req->req.length <= arm_addr->end) {
 				offset = req->req.address - arm_addr->start;
+				spin_unlock_irqrestore(&host_info_lock, flags);
 
 				DBGMSG
 				    ("arm_get_buf copy_to_user( %08X, %p, %u )",
 				     (u32) req->req.recvb,
 				     arm_addr->addr_space_buffer + offset,
 				     (u32) req->req.length);
-
 				if (copy_to_user
 				    (int2ptr(req->req.recvb),
 				     arm_addr->addr_space_buffer + offset,
-				     req->req.length)) {
-					spin_unlock_irqrestore(&host_info_lock,
-							       flags);
+				     req->req.length))
 					return (-EFAULT);
-				}
 
-				spin_unlock_irqrestore(&host_info_lock, flags);
 				/* We have to free the request, because we
 				 * queue no response, and therefore nobody
 				 * will free it. */
@@ -2005,24 +2026,23 @@ static int arm_set_buf(struct file_info *fi, struct pending_request *req)
 		    (arm_addr->end > req->req.address)) {
 			if (req->req.address + req->req.length <= arm_addr->end) {
 				offset = req->req.address - arm_addr->start;
+				spin_unlock_irqrestore(&host_info_lock, flags);
 
 				DBGMSG
 				    ("arm_set_buf copy_from_user( %p, %08X, %u )",
 				     arm_addr->addr_space_buffer + offset,
 				     (u32) req->req.sendb,
 				     (u32) req->req.length);
-
 				if (copy_from_user
 				    (arm_addr->addr_space_buffer + offset,
 				     int2ptr(req->req.sendb),
-				     req->req.length)) {
-					spin_unlock_irqrestore(&host_info_lock,
-							       flags);
+				     req->req.length))
 					return (-EFAULT);
-				}
 
-				spin_unlock_irqrestore(&host_info_lock, flags);
-				free_pending_request(req);	/* we have to free the request, because we queue no response, and therefore nobody will free it */
+				/* We have to free the request, because we
+				 * queue no response, and therefore nobody
+				 * will free it. */
+				free_pending_request(req);
 				return sizeof(struct raw1394_request);
 			} else {
 				DBGMSG("arm_set_buf request exceeded mapping");
@@ -2744,7 +2764,7 @@ static unsigned int raw1394_poll(struct file *file, poll_table * pt)
 	unsigned int mask = POLLOUT | POLLWRNORM;
 	unsigned long flags;
 
-	poll_wait(file, &fi->poll_wait_complete, pt);
+	poll_wait(file, &fi->wait_complete, pt);
 
 	spin_lock_irqsave(&fi->reqlists_lock, flags);
 	if (!list_empty(&fi->req_complete)) {
@@ -2769,9 +2789,8 @@ static int raw1394_open(struct inode *inode, struct file *file)
 	fi->state = opened;
 	INIT_LIST_HEAD(&fi->req_pending);
 	INIT_LIST_HEAD(&fi->req_complete);
-	sema_init(&fi->complete_sem, 0);
 	spin_lock_init(&fi->reqlists_lock);
-	init_waitqueue_head(&fi->poll_wait_complete);
+	init_waitqueue_head(&fi->wait_complete);
 	INIT_LIST_HEAD(&fi->addr_list);
 
 	file->private_data = fi;
@@ -2784,7 +2803,7 @@ static int raw1394_release(struct inode *inode, struct file *file)
 	struct file_info *fi = file->private_data;
 	struct list_head *lh;
 	struct pending_request *req;
-	int done = 0, i, fail = 0;
+	int i, fail;
 	int retval = 0;
 	struct list_head *entry;
 	struct arm_addr *addr = NULL;
@@ -2864,25 +2883,28 @@ static int raw1394_release(struct inode *inode, struct file *file)
 		       "error(s) occurred \n");
 	}
 
-	while (!done) {
+	for (;;) {
+		/* This locked section guarantees that neither
+		 * complete nor pending requests exist once i!=0 */
 		spin_lock_irqsave(&fi->reqlists_lock, flags);
-
-		while (!list_empty(&fi->req_complete)) {
-			lh = fi->req_complete.next;
-			list_del(lh);
-
-			req = list_entry(lh, struct pending_request, list);
-
+		while ((req = __next_complete_req(fi)))
 			free_pending_request(req);
-		}
-
-		if (list_empty(&fi->req_pending))
-			done = 1;
 
+		i = list_empty(&fi->req_pending);
 		spin_unlock_irqrestore(&fi->reqlists_lock, flags);
 
-		if (!done)
-			down_interruptible(&fi->complete_sem);
+		if (i)
+			break;
+		/*
+		 * Sleep until more requests can be freed.
+		 *
+		 * NB: We call the macro wait_event() with a condition argument
+		 * with side effect.  This is only possible because the side
+		 * effect does not occur until the condition became true, and
+		 * wait_event() won't evaluate the condition again after that.
+		 */
+		wait_event(fi->wait_complete, (req = next_complete_req(fi)));
+		free_pending_request(req);
 	}
 
 	/* Remove any sub-trees left by user space programs */