Skip to contentSkip to conversation

inotify: add FIONREAD support

PATCH In review Robert Love 3 participants 6 messages Nov 5, 2004 – Nov 7, 2004 Char and misc drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 inotify: add FIONREAD support

John,

There are a handful of "standard" file ioctl's (FIBMAP, FIONREAD, FIGETBSZ, etc.). We don't have to implement any of them, but FIONREAD is actually pretty useful: It tells you how many bytes are available on the fd.

This turns out to be needed because things listening to inotify during heavy file I/O can cause a scheduler scenario like

wake up, read 1 event, block
other guy runs, causes file I/O
wake up, read 1 event, block
other guy runs, causes file I/O
etc

And so disk intense operations like untarring take a bit longer because the scheduler behavior serializes the I/O operation, so to speak. This is a pathological case, and it relies on the scheduler semantics of the two processes, but it happens.

If Gamin or Beagle or whatever can "peak" at the fd and see it has only a small number of events available, they can yield or sleep for a few milliseconds, allowing the file I/O to continue, and not cause the scheduling mess.

So we see read events go from 1,1,1,1,1,... event read to something more acceptable, such as 200,200,200,... events read, and that is nice.

Attached patch adds the FIONREAD ioctl. It is only one line, really. ;-)

Best,

Robert Love

Add FIONREAD support to inotify. Walrus.

 drivers/char/inotify.c |    6 ++++++
 1 files changed, 6 insertions(+)
drivers/char/inotify.c +6 -0
diff -urN linux-2.6.10-rc1-inotify/drivers/char/inotify.c linux/drivers/char/inotify.c
@@ -35,6 +35,8 @@
 #include <linux/writeback.h>
 #include <linux/inotify.h>
 
+#include <asm/ioctls.h>
+
 static atomic_t watch_count;
 static atomic_t inotify_cookie;
 static kmem_cache_t *watch_cachep;
@@ -879,6 +881,7 @@
 	struct inotify_device *dev;
 	struct inotify_watch_request request;
 	void __user *p;
+	int bytes;
 	s32 wd;
 
 	dev = fp->private_data;
@@ -893,6 +896,9 @@
 		if (copy_from_user(&wd, p, sizeof (wd)))
 			return -EFAULT;
 		return inotify_ignore(dev, wd);
+	case FIONREAD:
+		bytes = dev->event_count * sizeof (struct inotify_event);
+		return put_user(bytes, (int *) p);
 	default:
 		return -ENOTTY;
 	}
#1 lore raw
Quoted 6 lines · Robert Love

John,

There are a handful of "standard" file ioctl's (FIBMAP, FIONREAD, FIGETBSZ, etc.). We don't have to implement any of them, but FIONREAD is actually pretty useful: It tells you how many bytes are available on the fd.

Nice idea.

drivers/char/inotify.c line 901

Quoted 3 lines
drivers/char/inotify.c +3 -0
+	case FIONREAD:
+		bytes = dev->event_count * sizeof (struct inotify_event);
+		return put_user(bytes, (int *) p);

But sparse will spit out warnings with code like this :(

Actually, the whole inotify patch probably isn't sparse clean...

thanks,

greg k-h

#2 lore raw
Quoted 1 line · Greg KH

But sparse will spit out warnings with code like this :(

Why? p is annotated __user.

Quoted 1 line

Actually, the whole inotify patch probably isn't sparse clean...

Probably not (I don't run sparse myself) but I did try to annotate all the user pointers with __user.

If you--or anyone else who uses sparse--sends me any warnings, I'll fix them.

Robert Love

#3 lore raw
Quoted 1 line · Robert Love

Why? p is annotated __user.

Oh, but I typecast that away. Doh.

Robert Love

Add FIONREAD support to inotify, take two. Strawberries.

 drivers/char/inotify.c |    6 ++++++
 1 files changed, 6 insertions(+)
drivers/char/inotify.c +6 -0
diff -urN linux-2.6.10-rc1-inotify/drivers/char/inotify.c linux/drivers/char/inotify.c
@@ -35,6 +35,8 @@
 #include <linux/writeback.h>
 #include <linux/inotify.h>
 
+#include <asm/ioctls.h>
+
 static atomic_t watch_count;
 static atomic_t inotify_cookie;
 static kmem_cache_t *watch_cachep;
@@ -879,6 +881,7 @@
 	struct inotify_device *dev;
 	struct inotify_watch_request request;
 	void __user *p;
+	int bytes;
 	s32 wd;
 
 	dev = fp->private_data;
@@ -893,6 +896,9 @@
 		if (copy_from_user(&wd, p, sizeof (wd)))
 			return -EFAULT;
 		return inotify_ignore(dev, wd);
+	case FIONREAD:
+		bytes = dev->event_count * sizeof (struct inotify_event);
+		return put_user(bytes, (int __user *) p);
 	default:
 		return -ENOTTY;
 	}
#4 lore raw

Looks good.

John

Quoted 47 lines · Robert Love
Quoted 1 line · Robert Love

Why? p is annotated __user.

Oh, but I typecast that away. Doh.

Robert Love

Add FIONREAD support to inotify, take two. Strawberries.

 drivers/char/inotify.c |    6 ++++++
 1 files changed, 6 insertions(+)

diff -urN linux-2.6.10-rc1-inotify/drivers/char/inotify.c linux/drivers/char/inotify.c

drivers/char/inotify.c +6 -0
@@ -35,6 +35,8 @@
 #include <linux/writeback.h>
 #include <linux/inotify.h>
 
+#include <asm/ioctls.h>
+
 static atomic_t watch_count;
 static atomic_t inotify_cookie;
 static kmem_cache_t *watch_cachep;
@@ -879,6 +881,7 @@
 	struct inotify_device *dev;
 	struct inotify_watch_request request;
 	void __user *p;
+	int bytes;
 	s32 wd;
 
 	dev = fp->private_data;
@@ -893,6 +896,9 @@
 		if (copy_from_user(&wd, p, sizeof (wd)))
 			return -EFAULT;
 		return inotify_ignore(dev, wd);
+	case FIONREAD:
+		bytes = dev->event_count * sizeof (struct inotify_event);
+		return put_user(bytes, (int __user *) p);
 	default:
 		return -ENOTTY;
 	}
Signature · 1 line
John McCutchan <[email protected]>
#5 lore raw
In reply to Robert Love
Quoted 5 lines · Robert Love
Quoted 1 line · Greg KH

But sparse will spit out warnings with code like this :(

Why? p is annotated __user.

Ah, ok, nevermind.

greg k-h