Re: [patch] inotify, improved.

[Zach Brown <zab@zabbo.net>]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]


> +		schedule();

Here's a stab at getting rid of that raw schedule() in inotify_read().
It maintains the behaviour where it returns when an event doesn't fit
and returns after events have been copied instead of sleeping.  It
changes behaviour in that it returns partial reads that suceeded instead
of the error that stopped processing.  It also lets threads who race out
of a wakeup to find an empty list go back to sleep instead of returning
0.  Dunno if that's behaviour you'd prefer but it seemed reasonable.  I
hope that lockless list_empty() is OK, I didn't think very hard about it.

Compiles but totally untested.  Check my work :)

- z

[-- Attachment #2: inotify-use-w-e-i-0.patch --]
[-- Type: text/x-patch, Size: 1868 bytes --]

Index: 2.6-mm-inotify-throwaway/fs/inotify.c
===================================================================
--- 2.6-mm-inotify-throwaway.orig/fs/inotify.c	2005-06-17 09:32:52.000000000 -0700
+++ 2.6-mm-inotify-throwaway/fs/inotify.c	2005-06-17 10:16:11.000000000 -0700
@@ -639,52 +639,32 @@
 static ssize_t inotify_read(struct file *file, char __user *buf,
 			    size_t count, loff_t *pos)
 {
-	size_t event_size = sizeof (struct inotify_event);
-	struct inotify_device *dev;
-	char __user *start;
-	int ret;
-	DEFINE_WAIT(wait);
-
-	start = buf;
-	dev = file->private_data;
-
-	while (1) {
-		int events;
-
-		prepare_to_wait(&dev->wq, &wait, TASK_INTERRUPTIBLE);
-
-		down(&dev->sem);
-		events = !list_empty(&dev->events);
-		up(&dev->sem);
-		if (events) {
-			ret = 0;
-			break;
-		}
-
-		if (file->f_flags & O_NONBLOCK) {
-			ret = -EAGAIN;
-			break;
-		}
-
-		if (signal_pending(current)) {
-			ret = -EINTR;
-			break;
-		}
-
-		schedule();
-	}
-
-	finish_wait(&dev->wq, &wait);
-	if (ret)
-		return ret;
+	struct inotify_device *dev = file->private_data;
+	char __user *start = buf;
+	int ret = 0;
 
 	down(&dev->sem);
 	while (1) {
 		struct inotify_kernel_event *kevent;
+		static size_t event_size = sizeof (struct inotify_event);
 
-		ret = buf - start;
-		if (list_empty(&dev->events))
-			break;
+		if (list_empty(&dev->events)) {
+			/* return partial instead of sleeping */
+			if (buf > start)
+				break;
+			if (file->f_flags & O_NONBLOCK) {
+				ret = -EAGAIN;
+				break;
+			}
+			up(&dev->sem);
+			ret = wait_event_interruptible(dev->wq,
+						!list_empty(&dev->events));
+			down(&dev->sem);
+			if (ret)
+				break;
+			continue;
+
+		}
 
 		kevent = inotify_dev_get_event(dev);
 		if (event_size + kevent->event.len > count)
@@ -710,6 +690,9 @@
 	}
 	up(&dev->sem);
 
+	if (buf > start)
+		ret = buf - start;
+
 	return ret;
 }