[patch] inotify: add FIONREAD support

[patch] inotify: add FIONREAD support

[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.

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(+)

diff -urN linux-2.6.10-rc1-inotify/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux-2.6.10-rc1-inotify/drivers/char/inotify.c	2004-11-05 17:26:52.182836608 -0500
+++ linux/drivers/char/inotify.c	2004-11-05 18:01:54.755197024 -0500
@@ -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;
 	}

Re: [patch] inotify: add FIONREAD support

[Greg KH]

On Fri, Nov 05, 2004 at 06:14:04PM -0500, Robert Love wrote:
> 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.

> +	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

Re: [patch] inotify: add FIONREAD support

[Robert Love]

On Fri, 2004-11-05 at 16:47 -0800, Greg KH wrote:

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

Why?  p is annotated __user.

> 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

Re: [patch] inotify: add FIONREAD support

[Robert Love]

On Fri, 2004-11-05 at 19:57 -0500, Robert Love wrote:

> 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
--- linux-2.6.10-rc1-inotify/drivers/char/inotify.c	2004-11-05 17:26:52.182836608 -0500
+++ linux/drivers/char/inotify.c	2004-11-05 18:01:54.755197024 -0500
@@ -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;
 	}

Re: [patch] inotify: add FIONREAD support

[John McCutchan]

Looks good.

John

On Fri, 2004-11-05 at 20:00 -0500, Robert Love wrote:
> On Fri, 2004-11-05 at 19:57 -0500, Robert Love wrote:
> 
> > 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
> --- linux-2.6.10-rc1-inotify/drivers/char/inotify.c	2004-11-05 17:26:52.182836608 -0500
> +++ linux/drivers/char/inotify.c	2004-11-05 18:01:54.755197024 -0500
> @@ -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;
>  	}
> 
> 
> 
-- 
John McCutchan <ttb@tentacle.dhs.org>

Re: [patch] inotify: add FIONREAD support

[Greg KH]

On Fri, Nov 05, 2004 at 07:57:43PM -0500, Robert Love wrote:
> On Fri, 2004-11-05 at 16:47 -0800, Greg KH wrote:
> 
> > But sparse will spit out warnings with code like this :(
> 
> Why?  p is annotated __user.

Ah, ok, nevermind.

greg k-h