[RFC][PATCH] inotify 0.9.2

[RFC][PATCH] inotify 0.9.2

[John McCutchan]

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

Hello,

Here is release 0.9.2 of inotify. Attached is a patch to 2.6.8.1

--New in this version--
Works with 4K stack kernels
Stops timers properly when using SMP

A couple of things to note, 

The memory usage analysis below is worst case, when inotify is watching
65536 inodes.
Inotify does pin inodes, but this is no different than dnotify.

Inotify is designed as a replacement for dnotify. 
The key difference's are:

Inotify does not require the file to be opened to watch it
When a path you are watching is unmounted you will recieve an UNMOUNT
event.
Events are devilered over a FD that is select()-able not with signals.


--COMPLEXITY--

I have been asked what the complexity of inotify is. Inotify has
2 path codes where complexity could be an issue:

Adding a watcher to a device
	This code has to check if the inode is already being watched 
	by the device, this is O(1) since the maximum number of 
	devices is limited to 8.


Removing a watch from a device
	This code has to do a search of all watches on the device to
	find the watch descriptor that is being asked to remove.
	This involves a linear search, but should not really be an issue
	because it is limited to 8192 entries. If this does turn in to
	a concern, I would replace the list of watches on the device
	with a sorted binary tree, so that the search could be done
	very quickly.


The calls to inotify from the VFS code has a complexity of O(1) so
inotify does not affect the speed of VFS operations.

--MEMORY USAGE--

The inotify data structures are light weight:

inotify watch is 40 bytes
inotify device is 68 bytes
inotify event is 272 bytes

So assuming a device has 8192 watches, the structures are only going
to consume 320KB of memory. With a maximum number of 8 devices allowed
to exist at a time, this is still only 2.5 MB

Each device can also have 256 events queued at a time, which sums to
68KB per device. And only .5 MB if all devices are opened and have
a full event queue.

So approximately 3 MB of memory are used in the rare case of 
everything open and full.

Each inotify watch pins the inode of a directory/file in memory,
the size of an inode is different per file system but lets assume
that it is 512 byes. 

So assuming the maximum number of global watches are active, this would
pin down 32 MB of inodes in the inode cache. Again not a problem
on a modern system. 

On smaller systems, the maximum watches / events could be lowered
to provide a smaller foot print.

Older release notes:
I am resubmitting inotify for comments and review. Inotify has
changed drastically from the earlier proposal that Al Viro did not
approve of. There is no longer any use of (device number, inode number)
pairs. Please give this version of inotify a fresh view.


Inotify is a character device that when opened offers 2 IOCTL's.
(It actually has 4 but the other 2 are used for debugging)

INOTIFY_WATCH:
        Which takes a path and event mask and returns a unique 
        (to the instance of the driver) integer (wd [watch descriptor]
        from here on) that is a 1:1 mapping to the path passed. 
        What happens is inotify gets the inode (and ref's the inode)
        for the path and adds a inotify_watcher structure to the inodes
        list of watchers. If this instance of the driver is already
        watching the path, the event mask will be updated and
        the original wd will be returned.

INOTIFY_IGNORE:
        Which takes an integer (that you got from INOTIFY_WATCH) 
        representing a wd that you are not interested in watching
        anymore. This will:

        send an IGNORE event to the device
        remove the inotify_watcher structure from the device and 
        from the inode and unref the inode.
        
After you are watching 1 or more paths, you can read from the fd
and get events. The events are struct inotify_event. If you are
watching a directory and something happens to a file in the directory
the event will contain the filename (just the filename not the full
path).

Aside from the inotify character device driver. 
The changes to the kernel are very minor. 

The first change is adding calls to inotify_inode_queue_event and
inotify_dentry_parent_queue_event from the various vfs functions. This
is identical to dnotify.

The second change is more serious, it adds a call to
inotify_super_block_umount
inside generic_shutdown_superblock. What inotify_super_block_umount does
is:

find all of the inodes that are on the super block being shut down,
sends each watcher on each inode the UNMOUNT and IGNORED event
removes the watcher structures from each instance of the device driver 
and each inode.
unref's the inode.

I would appreciate design review, code review and testing.

John


[-- Attachment #2: inotify-0.9.2.diff --]
[-- Type: text/x-patch, Size: 36591 bytes --]

diff -urN linux-2.6.8.1/drivers/char/inotify.c ../linux/drivers/char/inotify.c
--- linux-2.6.8.1/drivers/char/inotify.c	1969-12-31 19:00:00.000000000 -0500
+++ ../linux/drivers/char/inotify.c	2004-09-18 03:03:15.000000000 -0400
@@ -0,0 +1,977 @@
+/*
+ * Inode based directory notifications for Linux.
+ *
+ * Copyright (C) 2004 John McCutchan
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * Signed-off-by: John McCutchan ttb@tentacle.dhs.org
+ */
+
+/* TODO: 
+ * use rb tree so looking up watcher by watcher descriptor is faster.
+ * dev->watch_count is incremented twice for each watcher
+ */
+
+#include <linux/bitops.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <linux/poll.h>
+#include <linux/miscdevice.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/stddef.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/writeback.h>
+#include <linux/inotify.h>
+
+#define MAX_INOTIFY_DEVS 8 /* We only support X watchers */
+#define MAX_INOTIFY_DEV_WATCHERS 8192 /* A dev can only have Y watchers */
+#define MAX_INOTIFY_QUEUED_EVENTS 256 /* Only the first Z events will be queued */
+#define __BITMASK_SIZE (MAX_INOTIFY_DEV_WATCHERS / 8)
+
+#define INOTIFY_DEV_TIMER_TIME jiffies + (HZ/4)
+
+static atomic_t watcher_count; // < MAX_INOTIFY_DEVS
+
+static kmem_cache_t *watcher_cache;
+static kmem_cache_t *kevent_cache;
+
+/* For debugging */
+static int event_object_count;
+static int watcher_object_count;
+static int inode_ref_count;
+static int inotify_debug_flags;
+#define iprintk(f, str...) if (inotify_debug_flags & f) printk (KERN_ALERT str)
+
+/* For each inotify device we need to keep a list of events queued on it,
+ * a list of inodes that we are watching and other stuff.
+ */
+struct inotify_device {
+	struct list_head 	events;
+	atomic_t		event_count;
+	struct list_head 	watchers;
+	int			watcher_count;
+	wait_queue_head_t 	wait;
+	struct timer_list	timer;
+	char			read_state;
+	spinlock_t		lock;
+	void *			bitmask;
+};
+#define inotify_device_event_list(pos) list_entry((pos), struct inotify_event, list)
+
+struct inotify_watcher {
+	int 			wd; // watcher descriptor
+	unsigned long		mask;
+	struct inode *		inode;
+	struct inotify_device *	dev;
+	struct list_head	d_list; // device list
+	struct list_head	i_list; // inode list
+	struct list_head	u_list; // unmount list 
+};
+#define inotify_watcher_d_list(pos) list_entry((pos), struct inotify_watcher, d_list)
+#define inotify_watcher_i_list(pos) list_entry((pos), struct inotify_watcher, i_list)
+#define inotify_watcher_u_list(pos) list_entry((pos), struct inotify_watcher, u_list)
+
+/* A list of these is attached to each instance of the driver
+ * when the drivers read() gets called, this list is walked and
+ * all events that can fit in the buffer get delivered
+ */
+struct inotify_kernel_event {
+        struct list_head        list;
+	struct inotify_event	event;
+};
+#define list_to_inotify_kernel_event(pos) list_entry((pos), struct inotify_kernel_event, list)
+
+static int find_inode (const char __user *dirname, struct inode **inode)
+{
+	struct nameidata nd;
+	int error;
+
+	error = __user_walk (dirname, LOOKUP_FOLLOW, &nd);
+	if (error) {
+		iprintk(INOTIFY_DEBUG_INODE, "could not find inode\n");
+		goto out;
+	}
+
+	*inode = nd.dentry->d_inode;
+	__iget (*inode);
+	iprintk(INOTIFY_DEBUG_INODE, "ref'd inode\n");
+	inode_ref_count++;
+	path_release(&nd);
+out:
+	return error;
+}
+
+static void unref_inode (struct inode *inode) {
+	inode_ref_count--;
+	iprintk(INOTIFY_DEBUG_INODE, "unref'd inode\n");
+	iput (inode);
+}
+
+struct inotify_kernel_event *kernel_event (int wd, int mask, const char *filename) {
+	struct inotify_kernel_event *kevent;
+
+	kevent = kmem_cache_alloc (kevent_cache, GFP_ATOMIC);
+
+
+	if (!kevent) {
+		iprintk(INOTIFY_DEBUG_ALLOC, "failed to alloc kevent (%d,%d)\n", wd, mask);
+		goto out;
+	}
+
+	iprintk(INOTIFY_DEBUG_ALLOC, "alloced kevent %p (%d,%d)\n", kevent, wd, mask);
+
+	kevent->event.wd = wd;
+	kevent->event.mask = mask;
+	INIT_LIST_HEAD(&kevent->list);
+
+	if (filename) {
+		iprintk(INOTIFY_DEBUG_FILEN, "filename for event was %p %s\n", filename, filename);
+		strncpy (kevent->event.filename, filename, 256);
+		kevent->event.filename[255] = '\0';
+		iprintk(INOTIFY_DEBUG_FILEN, "filename after copying %s\n", kevent->event.filename);
+	} else {
+		iprintk(INOTIFY_DEBUG_FILEN, "no filename for event\n");
+		kevent->event.filename[0] = '\0';
+	}
+	
+	event_object_count++;
+
+out:
+	return kevent;
+}
+
+void delete_kernel_event (struct inotify_kernel_event *kevent) {
+	if (!kevent) return;
+
+	event_object_count--;
+
+	INIT_LIST_HEAD(&kevent->list);
+	kevent->event.wd = -1;
+	kevent->event.mask = 0;
+
+	iprintk(INOTIFY_DEBUG_ALLOC, "free'd kevent %p\n", kevent);
+	kmem_cache_free (kevent_cache, kevent);
+}
+
+#define inotify_dev_has_events(dev) (!list_empty(&dev->events))
+#define inotify_dev_get_event(dev) (list_to_inotify_kernel_event(dev->events.next))
+/* Does this events mask get sent to the watcher ? */
+#define event_and(event_mask,watchers_mask) 	((event_mask == IN_UNMOUNT) || \
+						(event_mask == IN_IGNORED) || \
+						(event_mask & watchers_mask))
+
+
+/* dev->lock == locked before calling */
+static void inotify_dev_queue_event (struct inotify_device *dev, struct inotify_watcher *watcher, int mask, const char *filename) {
+	struct inotify_kernel_event *kevent;
+
+	if (atomic_read(&dev->event_count) == MAX_INOTIFY_QUEUED_EVENTS) {
+		iprintk(INOTIFY_DEBUG_EVENTS, "event queue for %p overflowed\n", dev);
+		return;
+	}
+
+	if (!event_and(mask, watcher->inode->watchers_mask)||!event_and(mask, watcher->mask)) {
+		return;
+	}
+
+	atomic_inc(&dev->event_count);
+
+	kevent = kernel_event (watcher->wd, mask, filename);
+
+	if (!kevent) {
+		iprintk(INOTIFY_DEBUG_EVENTS, "failed to queue event %x for %p\n", mask, dev);
+	}
+
+	list_add_tail(&kevent->list, &dev->events);
+
+	iprintk(INOTIFY_DEBUG_EVENTS, "queued event %x for %p\n", mask, dev);
+}
+
+
+
+
+static void inotify_dev_event_dequeue (struct inotify_device *dev) {
+	struct inotify_kernel_event *kevent;
+
+	if (!inotify_dev_has_events (dev)) {
+		return;
+	}
+
+	kevent = inotify_dev_get_event(dev);
+
+	list_del(&kevent->list);
+	atomic_dec(&dev->event_count);
+
+	delete_kernel_event (kevent);
+
+	iprintk(INOTIFY_DEBUG_EVENTS, "dequeued event on %p\n", dev);
+}
+
+static int inotify_dev_get_wd (struct inotify_device *dev)
+{
+	int wd;
+
+	wd = -1;
+
+	if (!dev)
+		return -1;
+
+	if (dev->watcher_count == MAX_INOTIFY_DEV_WATCHERS) {
+		return -1;
+	}
+
+	dev->watcher_count++;
+
+	wd = find_first_zero_bit (dev->bitmask, __BITMASK_SIZE);
+
+	set_bit (wd, dev->bitmask);
+
+	return wd;
+}
+
+static int inotify_dev_put_wd (struct inotify_device *dev, int wd)
+{
+	if (!dev||wd < 0)
+		return -1;
+
+	dev->watcher_count--;
+
+	clear_bit (wd, dev->bitmask);
+
+	return 0;
+}
+
+
+static struct inotify_watcher *create_watcher (struct inotify_device *dev, int mask, struct inode *inode) {
+	struct inotify_watcher *watcher;
+
+	watcher = kmem_cache_alloc (watcher_cache, GFP_KERNEL);
+
+	if (!watcher) {
+		iprintk(INOTIFY_DEBUG_ALLOC, "failed to allocate watcher (%p,%d)\n", inode, mask);
+		return NULL;
+	}
+
+	watcher->wd = -1;
+	watcher->mask = mask;
+	watcher->inode = inode;
+	watcher->dev = dev;
+	INIT_LIST_HEAD(&watcher->d_list);
+	INIT_LIST_HEAD(&watcher->i_list);
+	INIT_LIST_HEAD(&watcher->u_list);
+
+	spin_lock(&dev->lock);
+		watcher->wd = inotify_dev_get_wd (dev);
+	spin_unlock(&dev->lock);
+
+	if (watcher->wd < 0) {
+		iprintk(INOTIFY_DEBUG_ERRORS, "Could not get wd for watcher %p\n", watcher);
+		iprintk(INOTIFY_DEBUG_ALLOC, "free'd watcher %p\n", watcher);
+		kmem_cache_free (watcher_cache, watcher);
+		watcher = NULL;
+		return watcher;
+	}
+
+	watcher_object_count++;
+	return watcher;
+}
+
+/* Must be called with dev->lock held */
+static void delete_watcher (struct inotify_device *dev, struct inotify_watcher *watcher) {
+	inotify_dev_put_wd (dev, watcher->wd);
+
+	iprintk(INOTIFY_DEBUG_ALLOC, "free'd watcher %p\n", watcher);
+
+	kmem_cache_free (watcher_cache, watcher);
+
+	watcher_object_count--;
+}
+
+
+static struct inotify_watcher *inode_find_dev (struct inode *inode, struct inotify_device *dev) {
+	struct inotify_watcher *watcher;
+
+	watcher = NULL;
+
+	list_for_each_entry (watcher, &inode->watchers, i_list) {
+		if (watcher->dev == dev) {
+			return watcher;
+		}
+
+	}
+	return NULL;
+}
+
+static struct inotify_watcher *dev_find_wd (struct inotify_device *dev, int wd)
+{
+	struct inotify_watcher *watcher;
+
+	list_for_each_entry (watcher, &dev->watchers, d_list) {
+		if (watcher->wd == wd) {
+			return watcher;
+		}
+	}
+	return NULL;
+}
+
+static int inotify_dev_is_watching_inode (struct inotify_device *dev, struct inode *inode) {
+	struct inotify_watcher *watcher;
+
+	list_for_each_entry (watcher, &dev->watchers, d_list) {
+		if (watcher->inode == inode) {
+			return 1;
+		}
+	}
+	
+	return 0;
+}
+
+static int inotify_dev_add_watcher (struct inotify_device *dev, struct inotify_watcher *watcher) {
+	int error;
+
+	error = 0;
+
+	if (!dev||!watcher) {
+		error = -EINVAL;
+		goto out;
+	}
+
+	if (dev_find_wd (dev, watcher->wd)) {
+		error = -EINVAL;
+		goto out;
+	}
+
+
+	if (dev->watcher_count == MAX_INOTIFY_DEV_WATCHERS) {
+		error = -ENOSPC;
+		goto out;
+	}
+
+	list_add(&watcher->d_list, &dev->watchers);
+out:
+	return error;
+}
+
+static int inotify_dev_rm_watcher (struct inotify_device *dev, struct inotify_watcher *watcher) {
+	int error;
+
+	error = -EINVAL;
+
+	if (watcher) {
+		inotify_dev_queue_event (dev, watcher, IN_IGNORED, NULL);
+
+		list_del(&watcher->d_list);
+
+		error = 0;
+	} 
+
+	return error;
+}
+
+void inode_update_watchers_mask (struct inode *inode)
+{
+	struct inotify_watcher *watcher;
+	unsigned long new_mask;
+
+	new_mask = 0;
+	list_for_each_entry(watcher, &inode->watchers, i_list) {
+		new_mask |= watcher->mask;
+	}
+	inode->watchers_mask = new_mask;
+}
+
+static int inode_add_watcher (struct inode *inode, struct inotify_watcher *watcher) {
+	if (!inode||!watcher||inode_find_dev (inode, watcher->dev))
+		return -EINVAL;
+
+	list_add(&watcher->i_list, &inode->watchers);
+	inode->watcher_count++;
+
+	inode_update_watchers_mask (inode);
+
+
+	return 0;
+}
+
+static int inode_rm_watcher (struct inode *inode, struct inotify_watcher *watcher) {
+	if (!inode||!watcher)
+		return -EINVAL;
+
+	list_del(&watcher->i_list);
+	inode->watcher_count--;
+
+	inode_update_watchers_mask (inode);
+
+	return 0;
+}
+
+/* Kernel API */
+
+void inotify_inode_queue_event (struct inode *inode, unsigned long mask, const char *filename) {
+	struct inotify_watcher *watcher;
+
+	spin_lock(&inode->i_lock);
+
+		list_for_each_entry (watcher, &inode->watchers, i_list) {
+			spin_lock(&watcher->dev->lock);
+				inotify_dev_queue_event (watcher->dev, watcher, mask, filename);
+			spin_unlock(&watcher->dev->lock);
+		}
+
+	spin_unlock(&inode->i_lock);
+}
+EXPORT_SYMBOL_GPL(inotify_inode_queue_event);
+
+void inotify_dentry_parent_queue_event(struct dentry *dentry, unsigned long mask, const char *filename)
+{
+	struct dentry *parent;
+
+	spin_lock(&dentry->d_lock);
+	dget (dentry->d_parent);
+	parent = dentry->d_parent;
+	inotify_inode_queue_event(parent->d_inode, mask, filename);
+	dput (parent);
+	spin_unlock(&dentry->d_lock);
+}
+EXPORT_SYMBOL_GPL(inotify_dentry_parent_queue_event);
+
+static void ignore_helper (struct inotify_watcher *watcher, int event) {
+	struct inotify_device *dev;
+	struct inode *inode;
+
+	spin_lock(&watcher->dev->lock);
+	spin_lock(&watcher->inode->i_lock);
+
+	inode = watcher->inode;
+	dev = watcher->dev;
+
+	if (event)
+		inotify_dev_queue_event (dev, watcher, event, NULL);
+
+	inode_rm_watcher (inode, watcher);
+	inotify_dev_rm_watcher (watcher->dev, watcher);
+	list_del(&watcher->u_list);
+
+
+	spin_unlock(&inode->i_lock);
+
+	delete_watcher(dev, watcher);
+
+	spin_unlock(&dev->lock);
+
+	unref_inode (inode);
+}
+
+static void process_umount_list (struct list_head *umount) {
+	struct inotify_watcher *watcher, *next;
+
+	list_for_each_entry_safe (watcher, next, umount, u_list) {
+		ignore_helper (watcher, IN_UNMOUNT);
+	}
+}
+
+static void build_umount_list (struct list_head *head, struct super_block *sb, struct list_head *umount) {
+	struct inode *	inode;
+
+	list_for_each_entry (inode, head, i_list) {
+		struct inotify_watcher *watcher;
+
+		if (inode->i_sb != sb)
+			continue;
+
+		spin_lock(&inode->i_lock);
+
+		list_for_each_entry (watcher, &inode->watchers, i_list) {
+			list_add (&watcher->u_list, umount);
+		}
+
+		spin_unlock(&inode->i_lock);
+	}
+}
+
+void inotify_super_block_umount (struct super_block *sb)
+{
+	struct list_head umount;
+
+	INIT_LIST_HEAD(&umount);
+
+	spin_lock(&inode_lock);
+		build_umount_list (&inode_in_use, sb, &umount);
+	spin_unlock(&inode_lock);
+
+	process_umount_list (&umount);
+}
+EXPORT_SYMBOL_GPL(inotify_super_block_umount);
+
+/* The driver interface is implemented below */
+
+static unsigned int inotify_poll(struct file *file, poll_table *wait) {
+        struct inotify_device *dev;
+
+        dev = file->private_data;
+
+
+        poll_wait(file, &dev->wait, wait);
+
+        if (inotify_dev_has_events(dev)) {
+                return POLLIN | POLLRDNORM;
+	}
+
+        return 0;
+}
+
+#define MAX_EVENTS_AT_ONCE 20
+static ssize_t inotify_read(struct file *file, __user char *buf,
+			   size_t count, loff_t *pos) {
+	size_t out;
+	struct inotify_event *eventbuf;
+	struct inotify_kernel_event *kevent;
+	struct inotify_device *dev;
+	char *obuf;
+	int err;
+	DECLARE_WAITQUEUE(wait, current);
+
+	int events;
+	int event_count;
+
+	eventbuf = kmalloc(sizeof(struct inotify_event) * MAX_EVENTS_AT_ONCE, 
+			GFP_KERNEL);
+	events = 0;
+	event_count = 0;
+	out = 0;
+	err = 0;
+
+	obuf = buf;
+
+	dev = file->private_data;
+
+	/* We only hand out full inotify events */
+	if (count < sizeof(struct inotify_event)) {
+		out = -EINVAL;
+		goto out;
+	}
+
+	events = count / sizeof(struct inotify_event);
+
+	if (events > MAX_EVENTS_AT_ONCE) events = MAX_EVENTS_AT_ONCE;
+
+	if (!inotify_dev_has_events(dev)) {
+		if (file->f_flags & O_NONBLOCK) {
+			out = -EAGAIN;
+			goto out;
+		}
+	}
+
+	spin_lock_irq(&dev->lock);
+
+	add_wait_queue(&dev->wait, &wait);
+repeat:
+	if (signal_pending(current)) {
+		spin_unlock_irq (&dev->lock);
+		out = -ERESTARTSYS;
+		set_current_state (TASK_RUNNING);
+		remove_wait_queue(&dev->wait, &wait);
+		goto out;
+	}
+	set_current_state(TASK_INTERRUPTIBLE);
+	if (!inotify_dev_has_events (dev)) {
+		spin_unlock_irq (&dev->lock);
+		schedule ();
+		spin_lock_irq (&dev->lock);
+		goto repeat;
+	}
+
+	set_current_state (TASK_RUNNING);
+	remove_wait_queue (&dev->wait, &wait);
+
+	err = !access_ok(VERIFY_WRITE, (void *)buf, sizeof(struct inotify_event));
+
+	if (err) {
+		out = -EFAULT;
+		goto out;
+	}
+
+
+	/* Copy all the events we can to the event buffer */
+	for (event_count = 0; event_count < events; event_count++) {
+		kevent = inotify_dev_get_event (dev);
+		eventbuf[event_count] = kevent->event;
+		inotify_dev_event_dequeue (dev);
+	}
+
+	spin_unlock_irq (&dev->lock);
+
+	/* Send the event buffer to user space */
+	err = copy_to_user (buf, eventbuf, events * sizeof(struct inotify_event));
+
+	buf += sizeof(struct inotify_event) * events;
+
+	out = buf - obuf;
+
+out:
+	kfree(eventbuf);
+	return out;
+}
+
+static void inotify_dev_timer (unsigned long data) {
+	struct inotify_device *dev = (struct inotify_device *)data;
+
+	if (!data) return;
+
+	// reset the timer
+	mod_timer(&dev->timer, INOTIFY_DEV_TIMER_TIME);
+
+	// wake up anything waiting on poll
+	if (inotify_dev_has_events (dev)) {
+		wake_up_interruptible(&dev->wait);
+	}
+}
+
+static int inotify_open(struct inode *inode, struct file *file) {
+	struct inotify_device *dev;
+
+	if (atomic_read(&watcher_count) == MAX_INOTIFY_DEVS)
+		return -ENODEV;
+
+	atomic_inc(&watcher_count);
+
+	dev = kmalloc(sizeof(struct inotify_device), GFP_KERNEL);
+	dev->bitmask = kmalloc(__BITMASK_SIZE, GFP_KERNEL);
+	memset(dev->bitmask, 0, __BITMASK_SIZE);
+
+	INIT_LIST_HEAD(&dev->events);
+	INIT_LIST_HEAD(&dev->watchers);
+	init_timer(&dev->timer);
+	init_waitqueue_head(&dev->wait);
+
+	atomic_set(&dev->event_count, 0);
+	dev->watcher_count = 0;
+	dev->lock = SPIN_LOCK_UNLOCKED;
+	dev->read_state = 0;
+
+	file->private_data = dev;
+
+	dev->timer.data = dev;
+	dev->timer.function = inotify_dev_timer;
+	dev->timer.expires = INOTIFY_DEV_TIMER_TIME;
+
+	add_timer(&dev->timer);
+
+	printk(KERN_ALERT "inotify device opened\n");
+
+	return 0;
+}
+
+static void inotify_release_all_watchers (struct inotify_device *dev)
+{
+	struct inotify_watcher *watcher,*next;
+
+	list_for_each_entry_safe(watcher, next, &dev->watchers, d_list) {
+		ignore_helper (watcher, 0);
+	}
+}
+
+static void inotify_release_all_events (struct inotify_device *dev)
+{
+	spin_lock(&dev->lock);
+	while (inotify_dev_has_events(dev)) {
+		inotify_dev_event_dequeue(dev);
+	}
+	spin_unlock(&dev->lock);
+}
+
+
+static int inotify_release(struct inode *inode, struct file *file)
+{
+	if (file->private_data) {
+		struct inotify_device *dev;
+
+		dev = (struct inotify_device *)file->private_data;
+
+		del_timer_sync(&dev->timer);
+
+		inotify_release_all_watchers(dev);
+
+		inotify_release_all_events(dev);
+
+		kfree (dev->bitmask);
+		kfree (dev);
+
+	}
+
+	printk(KERN_ALERT "inotify device released\n");
+
+	atomic_dec(&watcher_count);
+	return 0;
+}
+
+static int inotify_watch (struct inotify_device *dev, struct inotify_watch_request *request)
+{
+	int err;
+	struct inode *inode;
+	struct inotify_watcher *watcher;
+	err = 0;
+
+	err = find_inode (request->dirname, &inode);
+
+	if (err)
+		goto exit;
+
+	if (!S_ISDIR(inode->i_mode)) {
+		iprintk(INOTIFY_DEBUG_ERRORS, "watching file\n");
+	}
+
+	spin_lock(&dev->lock);
+	spin_lock(&inode->i_lock);
+
+	/* This handles the case of re-adding a directory we are already
+	 * watching, we just update the mask and return 0
+	 */
+	if (inotify_dev_is_watching_inode (dev, inode)) {
+		iprintk(INOTIFY_DEBUG_ERRORS, "adjusting event mask for inode %p\n", inode);
+		struct inotify_watcher *owatcher; // the old watcher
+
+		owatcher = inode_find_dev (inode, dev);
+
+		owatcher->mask = request->mask;
+
+		inode_update_watchers_mask (inode);
+
+		spin_unlock (&inode->i_lock);
+		spin_unlock (&dev->lock);
+
+		unref_inode (inode);
+
+		return 0;
+	}
+
+	spin_unlock (&inode->i_lock);
+	spin_unlock (&dev->lock);
+
+
+	watcher = create_watcher (dev, request->mask, inode);
+
+	if (!watcher) {
+		unref_inode (inode);
+		return -ENOSPC;
+	}
+
+	spin_lock(&dev->lock);
+	spin_lock(&inode->i_lock);
+
+	/* We can't add anymore watchers to this device */
+	if (inotify_dev_add_watcher (dev, watcher) == -ENOSPC) {
+		iprintk(INOTIFY_DEBUG_ERRORS, "can't add watcher dev is full\n");
+		spin_unlock (&inode->i_lock);
+		delete_watcher (dev, watcher);
+		spin_unlock (&dev->lock);
+
+		unref_inode (inode);
+		return -ENOSPC;
+	}
+
+	inode_add_watcher (inode, watcher);
+
+	/* We keep a reference on the inode */
+	if (!err) {
+		err = watcher->wd;
+	}
+
+	spin_unlock(&inode->i_lock);
+	spin_unlock(&dev->lock);
+exit:
+	return err;
+}
+
+static int inotify_ignore(struct inotify_device *dev, int wd)
+{
+	struct inotify_watcher *watcher;
+
+	watcher = dev_find_wd (dev, wd);
+
+	if (!watcher) {
+		return -EINVAL;
+	}
+
+	ignore_helper (watcher, 0);
+
+	return 0;
+}
+
+static void inotify_print_stats (struct inotify_device *dev)
+{
+	int sizeof_inotify_watcher;
+	int sizeof_inotify_device;
+	int sizeof_inotify_kernel_event;
+
+	sizeof_inotify_watcher = sizeof (struct inotify_watcher);
+	sizeof_inotify_device = sizeof (struct inotify_device);
+	sizeof_inotify_kernel_event = sizeof (struct inotify_kernel_event);
+
+	printk (KERN_ALERT "GLOBAL INOTIFY STATS\n");
+	printk (KERN_ALERT "watcher_count = %d\n", atomic_read(&watcher_count));
+	printk (KERN_ALERT "event_object_count = %d\n", event_object_count);
+	printk (KERN_ALERT "watcher_object_count = %d\n", watcher_object_count);
+	printk (KERN_ALERT "inode_ref_count = %d\n", inode_ref_count);
+
+	printk (KERN_ALERT "sizeof(struct inotify_watcher) = %d\n", sizeof_inotify_watcher);
+	printk (KERN_ALERT "sizeof(struct inotify_device) = %d\n", sizeof_inotify_device);
+	printk (KERN_ALERT "sizeof(struct inotify_kernel_event) = %d\n", sizeof_inotify_kernel_event);
+
+	spin_lock(&dev->lock);
+
+	printk (KERN_ALERT "inotify device: %p\n", dev);
+	printk (KERN_ALERT "inotify event_count: %d\n", atomic_read(&dev->event_count));
+	printk (KERN_ALERT "inotify watch_count: %d\n", dev->watcher_count);
+
+	spin_unlock(&dev->lock);
+}
+
+static int inotify_ioctl(struct inode *ip, struct file *fp,
+			 unsigned int cmd, unsigned long arg) {
+	int err;
+	struct inotify_device *dev;
+	struct inotify_watch_request request;
+	int wid;
+
+	dev = fp->private_data;
+
+	err = 0;
+
+	if (_IOC_TYPE(cmd) != INOTIFY_IOCTL_MAGIC) return -EINVAL;
+	if (_IOC_NR(cmd) > INOTIFY_IOCTL_MAXNR) return -EINVAL;
+
+	if (_IOC_DIR(cmd) & _IOC_READ)
+		err = !access_ok(VERIFY_READ, (void *)arg, _IOC_SIZE(cmd));
+
+	if (err) {
+		err = -EFAULT;
+		goto out;
+	}
+
+	if (_IOC_DIR(cmd) & _IOC_WRITE)
+		err = !access_ok(VERIFY_WRITE, (void *)arg, _IOC_SIZE(cmd));
+
+	if (err) {
+		err = -EFAULT;
+		goto out;
+	}
+
+
+	err = -EINVAL;
+
+	switch (cmd) {
+		case INOTIFY_WATCH:
+			iprintk(INOTIFY_DEBUG_ERRORS, "INOTIFY_WATCH ioctl\n");
+			if (copy_from_user(&request, (void *)arg, sizeof(struct inotify_watch_request))) {
+				err = -EFAULT;
+				goto out;
+			}
+
+			err = inotify_watch(dev, &request);
+		break;
+		case INOTIFY_IGNORE:
+			iprintk(INOTIFY_DEBUG_ERRORS, "INOTIFY_IGNORE ioctl\n");
+			if (copy_from_user(&wid, (void *)arg, sizeof(int))) {
+				err = -EFAULT;
+				goto out;
+			}
+
+			err = inotify_ignore(dev, wid);
+		break;
+		case INOTIFY_STATS:
+			iprintk(INOTIFY_DEBUG_ERRORS, "INOTIFY_STATS ioctl\n");
+			inotify_print_stats (dev);
+			err = 0;
+		break;
+		case INOTIFY_SETDEBUG:
+			iprintk(INOTIFY_DEBUG_ERRORS, "INOTIFY_SETDEBUG ioctl\n");
+			if (copy_from_user(&inotify_debug_flags, (void *)arg, sizeof(int))) {
+				err = -EFAULT;
+				goto out;
+			}
+		break;
+	}
+
+out:
+	return err;
+}
+
+static struct file_operations inotify_fops = {
+	.owner		= THIS_MODULE,
+	.poll		= inotify_poll,
+	.read		= inotify_read,
+	.open		= inotify_open,
+	.release	= inotify_release,
+	.ioctl		= inotify_ioctl,
+};
+
+struct miscdevice inotify_device = {
+	.minor  = MISC_DYNAMIC_MINOR, // automatic
+	.name	= "inotify",
+	.fops	= &inotify_fops,
+};
+
+
+static int __init inotify_init (void)
+{
+	int ret;
+
+	ret = misc_register(&inotify_device);
+
+	if (ret) {
+		goto out;
+	}
+
+	inotify_debug_flags = INOTIFY_DEBUG_NONE;
+
+	watcher_cache = kmem_cache_create ("watcher_cache", 
+			sizeof(struct inotify_watcher), 0, SLAB_PANIC, NULL, NULL);
+
+	if (!watcher_cache) {
+		misc_deregister (&inotify_device);
+	}
+	kevent_cache = kmem_cache_create ("kevent_cache", 
+			sizeof(struct inotify_kernel_event), 0, SLAB_PANIC, NULL, NULL);
+
+	if (!kevent_cache) {
+		misc_deregister (&inotify_device);
+		kmem_cache_destroy (watcher_cache);
+	}
+
+	printk(KERN_ALERT "inotify 0.9.2 minor=%d\n", inotify_device.minor);
+out:
+	return ret;
+}
+
+static void inotify_exit (void)
+{
+	kmem_cache_destroy (kevent_cache);
+	kmem_cache_destroy (watcher_cache);
+	misc_deregister (&inotify_device);
+	printk(KERN_ALERT "inotify shutdown ec=%d wc=%d ic=%d\n", event_object_count, watcher_object_count, inode_ref_count);
+}
+
+MODULE_AUTHOR("John McCutchan <ttb@tentacle.dhs.org>");
+MODULE_DESCRIPTION("Inode event driver");
+MODULE_LICENSE("GPL");
+
+module_init (inotify_init);
+module_exit (inotify_exit);
diff -urN linux-2.6.8.1/drivers/char/Makefile ../linux/drivers/char/Makefile
--- linux-2.6.8.1/drivers/char/Makefile	2004-08-14 06:56:22.000000000 -0400
+++ ../linux/drivers/char/Makefile	2004-08-19 00:11:52.000000000 -0400
@@ -7,7 +7,7 @@
 #
 FONTMAPFILE = cp437.uni
 
-obj-y	 += mem.o random.o tty_io.o n_tty.o tty_ioctl.o pty.o misc.o
+obj-y	 += mem.o random.o tty_io.o n_tty.o tty_ioctl.o pty.o misc.o inotify.o
 
 obj-$(CONFIG_VT)		+= vt_ioctl.o vc_screen.o consolemap.o \
 				   consolemap_deftbl.o selection.o keyboard.o
diff -urN linux-2.6.8.1/fs/attr.c ../linux/fs/attr.c
--- linux-2.6.8.1/fs/attr.c	2004-08-14 06:54:50.000000000 -0400
+++ ../linux/fs/attr.c	2004-08-19 00:11:52.000000000 -0400
@@ -11,6 +11,7 @@
 #include <linux/string.h>
 #include <linux/smp_lock.h>
 #include <linux/dnotify.h>
+#include <linux/inotify.h>
 #include <linux/fcntl.h>
 #include <linux/quotaops.h>
 #include <linux/security.h>
@@ -185,8 +186,11 @@
 	}
 	if (!error) {
 		unsigned long dn_mask = setattr_mask(ia_valid);
-		if (dn_mask)
+		if (dn_mask) {
 			dnotify_parent(dentry, dn_mask);
+			inotify_inode_queue_event (dentry->d_inode, dn_mask, NULL);
+			inotify_dentry_parent_queue_event (dentry, dn_mask, dentry->d_name.name);
+		}
 	}
 	return error;
 }
diff -urN linux-2.6.8.1/fs/inode.c ../linux/fs/inode.c
--- linux-2.6.8.1/fs/inode.c	2004-08-14 06:56:23.000000000 -0400
+++ ../linux/fs/inode.c	2004-08-19 00:11:52.000000000 -0400
@@ -114,6 +114,7 @@
 	if (inode) {
 		struct address_space * const mapping = &inode->i_data;
 
+		INIT_LIST_HEAD (&inode->watchers);
 		inode->i_sb = sb;
 		inode->i_blkbits = sb->s_blocksize_bits;
 		inode->i_flags = 0;
diff -urN linux-2.6.8.1/fs/namei.c ../linux/fs/namei.c
--- linux-2.6.8.1/fs/namei.c	2004-08-14 06:55:10.000000000 -0400
+++ ../linux/fs/namei.c	2004-09-15 16:43:23.000000000 -0400
@@ -22,6 +22,7 @@
 #include <linux/quotaops.h>
 #include <linux/pagemap.h>
 #include <linux/dnotify.h>
+#include <linux/inotify.h>
 #include <linux/smp_lock.h>
 #include <linux/personality.h>
 #include <linux/security.h>
@@ -1221,6 +1222,7 @@
 	error = dir->i_op->create(dir, dentry, mode, nd);
 	if (!error) {
 		inode_dir_notify(dir, DN_CREATE);
+		inotify_inode_queue_event(dir, IN_CREATE, dentry->d_name.name);
 		security_inode_post_create(dir, dentry, mode);
 	}
 	return error;
@@ -1535,6 +1537,7 @@
 	error = dir->i_op->mknod(dir, dentry, mode, dev);
 	if (!error) {
 		inode_dir_notify(dir, DN_CREATE);
+		inotify_inode_queue_event(dir, IN_CREATE, dentry->d_name.name);
 		security_inode_post_mknod(dir, dentry, mode, dev);
 	}
 	return error;
@@ -1608,6 +1611,7 @@
 	error = dir->i_op->mkdir(dir, dentry, mode);
 	if (!error) {
 		inode_dir_notify(dir, DN_CREATE);
+		inotify_inode_queue_event(dir, IN_CREATE, dentry->d_name.name);
 		security_inode_post_mkdir(dir,dentry, mode);
 	}
 	return error;
@@ -1703,6 +1707,8 @@
 	up(&dentry->d_inode->i_sem);
 	if (!error) {
 		inode_dir_notify(dir, DN_DELETE);
+		inotify_inode_queue_event(dir, IN_DELETE, dentry->d_name.name);
+		inotify_inode_queue_event(dentry->d_inode, IN_DELETE, NULL);
 		d_delete(dentry);
 	}
 	dput(dentry);
@@ -1775,8 +1781,9 @@
 
 	/* We don't d_delete() NFS sillyrenamed files--they still exist. */
 	if (!error && !(dentry->d_flags & DCACHE_NFSFS_RENAMED)) {
-		d_delete(dentry);
 		inode_dir_notify(dir, DN_DELETE);
+		inotify_inode_queue_event(dir, IN_DELETE, dentry->d_name.name);
+		d_delete(dentry);
 	}
 	return error;
 }
@@ -1853,6 +1860,7 @@
 	error = dir->i_op->symlink(dir, dentry, oldname);
 	if (!error) {
 		inode_dir_notify(dir, DN_CREATE);
+		inotify_inode_queue_event(dir, IN_CREATE, dentry->d_name.name);
 		security_inode_post_symlink(dir, dentry, oldname);
 	}
 	return error;
@@ -1926,6 +1934,7 @@
 	up(&old_dentry->d_inode->i_sem);
 	if (!error) {
 		inode_dir_notify(dir, DN_CREATE);
+		inotify_inode_queue_event(dir, IN_CREATE, new_dentry->d_name.name);
 		security_inode_post_link(old_dentry, dir, new_dentry);
 	}
 	return error;
@@ -2115,12 +2124,14 @@
 	else
 		error = vfs_rename_other(old_dir,old_dentry,new_dir,new_dentry);
 	if (!error) {
-		if (old_dir == new_dir)
+		if (old_dir == new_dir) {
 			inode_dir_notify(old_dir, DN_RENAME);
-		else {
+		} else {
 			inode_dir_notify(old_dir, DN_DELETE);
 			inode_dir_notify(new_dir, DN_CREATE);
 		}
+
+		inotify_inode_queue_event(old_dir, IN_RENAME, new_dentry->d_name.name);
 	}
 	return error;
 }
diff -urN linux-2.6.8.1/fs/open.c ../linux/fs/open.c
--- linux-2.6.8.1/fs/open.c	2004-08-14 06:54:48.000000000 -0400
+++ ../linux/fs/open.c	2004-08-19 00:11:52.000000000 -0400
@@ -11,6 +11,7 @@
 #include <linux/smp_lock.h>
 #include <linux/quotaops.h>
 #include <linux/dnotify.h>
+#include <linux/inotify.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/tty.h>
@@ -955,6 +956,8 @@
 			error = PTR_ERR(f);
 			if (IS_ERR(f))
 				goto out_error;
+			inotify_inode_queue_event (f->f_dentry->d_inode, IN_OPEN, NULL);
+			inotify_dentry_parent_queue_event (f->f_dentry, IN_OPEN, f->f_dentry->d_name.name);
 			fd_install(fd, f);
 		}
 out:
@@ -1034,6 +1037,8 @@
 	FD_CLR(fd, files->close_on_exec);
 	__put_unused_fd(files, fd);
 	spin_unlock(&files->file_lock);
+	inotify_dentry_parent_queue_event (filp->f_dentry, IN_CLOSE, filp->f_dentry->d_name.name);
+	inotify_inode_queue_event (filp->f_dentry->d_inode, IN_CLOSE, NULL);
 	return filp_close(filp, files);
 
 out_unlock:
diff -urN linux-2.6.8.1/fs/read_write.c ../linux/fs/read_write.c
--- linux-2.6.8.1/fs/read_write.c	2004-08-14 06:55:35.000000000 -0400
+++ ../linux/fs/read_write.c	2004-08-19 00:11:52.000000000 -0400
@@ -11,6 +11,7 @@
 #include <linux/uio.h>
 #include <linux/smp_lock.h>
 #include <linux/dnotify.h>
+#include <linux/inotify.h>
 #include <linux/security.h>
 #include <linux/module.h>
 
@@ -216,8 +217,11 @@
 				ret = file->f_op->read(file, buf, count, pos);
 			else
 				ret = do_sync_read(file, buf, count, pos);
-			if (ret > 0)
+			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_ACCESS);
+				inotify_dentry_parent_queue_event(file->f_dentry, IN_ACCESS, file->f_dentry->d_name.name);
+				inotify_inode_queue_event (file->f_dentry->d_inode, IN_ACCESS, NULL);
+			}
 		}
 	}
 
@@ -260,8 +264,11 @@
 				ret = file->f_op->write(file, buf, count, pos);
 			else
 				ret = do_sync_write(file, buf, count, pos);
-			if (ret > 0)
+			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_MODIFY);
+				inotify_dentry_parent_queue_event(file->f_dentry, IN_MODIFY, file->f_dentry->d_name.name);
+				inotify_inode_queue_event (file->f_dentry->d_inode, IN_MODIFY, NULL);
+			}
 		}
 	}
 
@@ -493,9 +500,13 @@
 out:
 	if (iov != iovstack)
 		kfree(iov);
-	if ((ret + (type == READ)) > 0)
+	if ((ret + (type == READ)) > 0) {
 		dnotify_parent(file->f_dentry,
 				(type == READ) ? DN_ACCESS : DN_MODIFY);
+		inotify_dentry_parent_queue_event(file->f_dentry, 
+				(type == READ) ? IN_ACCESS : IN_MODIFY, file->f_dentry->d_name.name);
+		inotify_inode_queue_event (file->f_dentry->d_inode, (type == READ) ? IN_ACCESS : IN_MODIFY, NULL);
+	}
 	return ret;
 }
 
diff -urN linux-2.6.8.1/fs/super.c ../linux/fs/super.c
--- linux-2.6.8.1/fs/super.c	2004-08-14 06:55:22.000000000 -0400
+++ ../linux/fs/super.c	2004-09-18 02:24:33.000000000 -0400
@@ -36,6 +36,7 @@
 #include <linux/writeback.h>		/* for the emergency remount stuff */
 #include <linux/idr.h>
 #include <asm/uaccess.h>
+#include <linux/inotify.h>
 
 
 void get_filesystem(struct file_system_type *fs);
@@ -204,6 +205,7 @@
 
 	if (root) {
 		sb->s_root = NULL;
+		inotify_super_block_umount (sb);
 		shrink_dcache_parent(root);
 		shrink_dcache_anon(&sb->s_anon);
 		dput(root);
diff -urN linux-2.6.8.1/include/linux/fs.h ../linux/include/linux/fs.h
--- linux-2.6.8.1/include/linux/fs.h	2004-08-14 06:55:09.000000000 -0400
+++ ../linux/include/linux/fs.h	2004-09-18 02:24:33.000000000 -0400
@@ -458,6 +458,10 @@
 	unsigned long		i_dnotify_mask; /* Directory notify events */
 	struct dnotify_struct	*i_dnotify; /* for directory notifications */
 
+	struct list_head	watchers;
+	unsigned long		watchers_mask;
+	int			watcher_count;
+
 	unsigned long		i_state;
 	unsigned long		dirtied_when;	/* jiffies of first dirtying */
 
diff -urN linux-2.6.8.1/include/linux/inotify.h ../linux/include/linux/inotify.h
--- linux-2.6.8.1/include/linux/inotify.h	1969-12-31 19:00:00.000000000 -0500
+++ ../linux/include/linux/inotify.h	2004-08-19 00:11:52.000000000 -0400
@@ -0,0 +1,74 @@
+/*
+ * Inode based directory notification for Linux
+ *
+ * Copyright (C) 2004 John McCutchan
+ *
+ * Signed-off-by: John McCutchan ttb@tentacle.dhs.org
+ */
+
+#ifndef _LINUX_INOTIFY_H
+#define _LINUX_INOTIFY_H
+
+struct inode;
+struct dentry;
+struct super_block;
+
+struct inotify_event {
+	int wd;
+	int mask;
+	char filename[256];
+	/* When you are watching a directory you will get the filenames
+	 * for events like IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, etc.. 
+	 */
+};
+/* When reading from the device you must provide a buffer 
+ * that is a multiple of the sizeof(inotify_event)
+ */
+
+#define IN_ACCESS	0x00000001	/* File was accessed */
+#define IN_MODIFY	0x00000002	/* File was modified */
+#define IN_CREATE	0x00000004	/* File was created */
+#define IN_DELETE	0x00000008	/* File was deleted */
+#define IN_RENAME	0x00000010	/* File was renamed */
+#define IN_ATTRIB	0x00000020	/* File changed attributes */
+#define IN_MOVE		0x00000040	/* File was moved */
+#define IN_UNMOUNT	0x00000080	/* Device file was on, was unmounted */
+#define IN_CLOSE	0x00000100	/* File was closed */
+#define IN_OPEN		0x00000200	/* File was opened */
+#define IN_IGNORED	0x00000400	/* File was ignored */
+#define IN_ALL_EVENTS	0xffffffff	/* All the events */
+
+/* ioctl */
+
+/* Fill this and pass it to INOTIFY_WATCH ioctl */
+struct inotify_watch_request {
+	char *dirname; // directory name
+	unsigned long mask; // event mask
+};
+
+#define INOTIFY_IOCTL_MAGIC 'Q'
+#define INOTIFY_IOCTL_MAXNR 4
+
+#define INOTIFY_WATCH  		_IOR(INOTIFY_IOCTL_MAGIC, 1, struct inotify_watch_request)
+#define INOTIFY_IGNORE 		_IOR(INOTIFY_IOCTL_MAGIC, 2, int)
+#define INOTIFY_STATS		_IOR(INOTIFY_IOCTL_MAGIC, 3, int)
+#define INOTIFY_SETDEBUG	_IOR(INOTIFY_IOCTL_MAGIC, 4, int)
+
+#define INOTIFY_DEBUG_NONE   0x00000000
+#define INOTIFY_DEBUG_ALLOC  0x00000001
+#define INOTIFY_DEBUG_EVENTS 0x00000002
+#define INOTIFY_DEBUG_INODE  0x00000004
+#define INOTIFY_DEBUG_ERRORS 0x00000008
+#define INOTIFY_DEBUG_FILEN  0x00000010
+#define INOTIFY_DEBUG_ALL    0xffffffff
+
+/* Kernel API */
+/* Adds events to all watchers on inode that are interested in mask */
+void inotify_inode_queue_event (struct inode *inode, unsigned long mask, const char *filename);
+/* Same as above but uses dentry's inode */
+void inotify_dentry_parent_queue_event (struct dentry *dentry, unsigned long mask, const char *filename);
+/* This will remove all watchers from all inodes on the superblock */
+void inotify_super_block_umount (struct super_block *sb);
+
+#endif
+

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

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

On Sun, 2004-09-19 at 23:56 -0400, John McCutchan wrote:

> I would appreciate design review, code review and testing.

Hi, John.

When you pass SLAB_PANIC to kmem_cache_create(), the slab layer will
panic the kernel and thus halt the machine.  So there is no reason to
check the return value.

We could remove the SLAB_PANIC, but I think that it makes sense, so
instead I removed the code checking the returns, saving a few bytes.

Patch is against your inotify patch.

	Robert Love


[-- Attachment #2: inotify-slab-panic-rml-2.6.9-rc2.patch --]
[-- Type: text/x-patch, Size: 1184 bytes --]

Signed-Off-By: Robert Love <rml@novell.com>

 drivers/char/inotify.c |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)

diff -urN linux-inotify/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux-inotify/drivers/char/inotify.c	2004-09-20 17:10:58.000000000 -0400
+++ linux/drivers/char/inotify.c	2004-09-20 17:33:03.369317992 -0400
@@ -942,19 +942,13 @@
 
 	inotify_debug_flags = INOTIFY_DEBUG_NONE;
 
-	watcher_cache = kmem_cache_create ("watcher_cache", 
-			sizeof(struct inotify_watcher), 0, SLAB_PANIC, NULL, NULL);
-
-	if (!watcher_cache) {
-		misc_deregister (&inotify_device);
-	}
-	kevent_cache = kmem_cache_create ("kevent_cache", 
-			sizeof(struct inotify_kernel_event), 0, SLAB_PANIC, NULL, NULL);
-
-	if (!kevent_cache) {
-		misc_deregister (&inotify_device);
-		kmem_cache_destroy (watcher_cache);
-	}
+	watcher_cache = kmem_cache_create ("watcher_cache",
+			sizeof(struct inotify_watcher), 0,
+			SLAB_PANIC, NULL, NULL);
+
+	kevent_cache = kmem_cache_create ("kevent_cache",
+			sizeof(struct inotify_kernel_event), 0,
+			SLAB_PANIC, NULL, NULL);
 
 	printk(KERN_ALERT "inotify 0.9.2 minor=%d\n", inotify_device.minor);
 out:

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

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

On Sun, 2004-09-19 at 23:56 -0400, John McCutchan wrote:

> I would appreciate design review, code review and testing.

Hey, John!

Attached patch, against inotify 0.9.2, cleans up the inotify.h header
file a bit.  The patch is a little noisy because I reformat some lines
for readability or coding style concerns, but there are a few important
changes:

	- Use PATH_MAX, the actual maximum path length, not 256, for the
	  size of "filename" in "struct inotify_event"

	- Separate with __KERNEL__ the kernel from the user API

	- Instead of forward declaring the inode, superblock, and
	  dentry structures, include the appropriate header file

Thanks!

	Robert Love


[-- Attachment #2: inotify-cleanup-header-rml-1.patch --]
[-- Type: text/x-patch, Size: 4390 bytes --]

Clean up the inotify header

Signed-Off-By: Robert Love <rml@novell.com>

 include/linux/inotify.h |   79 +++++++++++++++++++++++++++++-------------------
 1 files changed, 48 insertions(+), 31 deletions(-)

--- linux-inotify/include/linux/inotify.h	2004-09-21 00:58:03.920388608 -0400
+++ linux/include/linux/inotify.h	2004-09-21 01:15:05.831034576 -0400
@@ -9,66 +9,83 @@
 #ifndef _LINUX_INOTIFY_H
 #define _LINUX_INOTIFY_H
 
-struct inode;
-struct dentry;
-struct super_block;
+#include <linux/limits.h>
 
+/*
+ * struct inotify_event - structure read from the inotify device for each event
+ *
+ * When you are watching a directory, you will receive the filename for events
+ * such as IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, and so on ...
+ *
+ * Note: When reading from the device you must provide a buffer that is a
+ * multiple of sizeof(struct inotify_event)
+ */
 struct inotify_event {
 	int wd;
 	int mask;
-	char filename[256];
-	/* When you are watching a directory you will get the filenames
-	 * for events like IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, etc.. 
-	 */
+	char filename[PATH_MAX];
 };
-/* When reading from the device you must provide a buffer 
- * that is a multiple of the sizeof(inotify_event)
- */
 
+/* the following are legal, implemented events */
 #define IN_ACCESS	0x00000001	/* File was accessed */
 #define IN_MODIFY	0x00000002	/* File was modified */
 #define IN_CREATE	0x00000004	/* File was created */
 #define IN_DELETE	0x00000008	/* File was deleted */
 #define IN_RENAME	0x00000010	/* File was renamed */
-#define IN_ATTRIB	0x00000020	/* File changed attributes */
-#define IN_MOVE		0x00000040	/* File was moved */
-#define IN_UNMOUNT	0x00000080	/* Device file was on, was unmounted */
+#define IN_UNMOUNT	0x00000080	/* Backing filesystem was unmounted */
 #define IN_CLOSE	0x00000100	/* File was closed */
 #define IN_OPEN		0x00000200	/* File was opened */
+
+/* the following are legal, but not yet implemented, events */
+#define IN_ATTRIB	0x00000020	/* File changed attributes */
+#define IN_MOVE		0x00000040	/* File was moved */
+
+/* special flags */
 #define IN_IGNORED	0x00000400	/* File was ignored */
 #define IN_ALL_EVENTS	0xffffffff	/* All the events */
 
-/* ioctl */
-
-/* Fill this and pass it to INOTIFY_WATCH ioctl */
+/*
+ * struct inotify_watch_request - represents a watch request
+ *
+ * Pass to the inotify device via the INOTIFY_WATCH ioctl
+ */
 struct inotify_watch_request {
-	char *dirname; // directory name
-	unsigned long mask; // event mask
+	char *dirname;		/* directory name */
+	unsigned long mask;	/* event mask */
 };
 
-#define INOTIFY_IOCTL_MAGIC 'Q'
-#define INOTIFY_IOCTL_MAXNR 4
+#define INOTIFY_IOCTL_MAGIC	'Q'
+#define INOTIFY_IOCTL_MAXNR	4
 
 #define INOTIFY_WATCH  		_IOR(INOTIFY_IOCTL_MAGIC, 1, struct inotify_watch_request)
 #define INOTIFY_IGNORE 		_IOR(INOTIFY_IOCTL_MAGIC, 2, int)
 #define INOTIFY_STATS		_IOR(INOTIFY_IOCTL_MAGIC, 3, int)
 #define INOTIFY_SETDEBUG	_IOR(INOTIFY_IOCTL_MAGIC, 4, int)
 
-#define INOTIFY_DEBUG_NONE   0x00000000
-#define INOTIFY_DEBUG_ALLOC  0x00000001
-#define INOTIFY_DEBUG_EVENTS 0x00000002
-#define INOTIFY_DEBUG_INODE  0x00000004
-#define INOTIFY_DEBUG_ERRORS 0x00000008
-#define INOTIFY_DEBUG_FILEN  0x00000010
-#define INOTIFY_DEBUG_ALL    0xffffffff
+#define INOTIFY_DEBUG_NONE	0x00000000
+#define INOTIFY_DEBUG_ALLOC	0x00000001
+#define INOTIFY_DEBUG_EVENTS	0x00000002
+#define INOTIFY_DEBUG_INODE	0x00000004
+#define INOTIFY_DEBUG_ERRORS	0x00000008
+#define INOTIFY_DEBUG_FILEN	0x00000010
+#define INOTIFY_DEBUG_ALL	0xffffffff
+
+#ifdef __KERNEL__
+
+#include <linux/dcache.h>
+#include <linux/fs.h>
 
-/* Kernel API */
 /* Adds events to all watchers on inode that are interested in mask */
-void inotify_inode_queue_event (struct inode *inode, unsigned long mask, const char *filename);
+void inotify_inode_queue_event (struct inode *inode, unsigned long mask,
+		const char *filename);
+
 /* Same as above but uses dentry's inode */
-void inotify_dentry_parent_queue_event (struct dentry *dentry, unsigned long mask, const char *filename);
+void inotify_dentry_parent_queue_event (struct dentry *dentry,
+		unsigned long mask, const char *filename);
+
 /* This will remove all watchers from all inodes on the superblock */
 void inotify_super_block_umount (struct super_block *sb);
 
-#endif
+#endif	/* __KERNEL __ */
 
+#endif	/* _LINUX_INOTIFY_H */

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

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

On Sun, 2004-09-19 at 23:56 -0400, John McCutchan wrote:

> I would appreciate design review, code review and testing.

Hi, John.

Attached patches fixes two compile warnings I receive in
drivers/char/inotify.c:

	- Declaration after code in inotify_watch()

	- Uncasted conversion from pointer to integer

Also, wrap some arithmetic in parenthesis, to be safe.

Best,

	Robert Love


[-- Attachment #2: inotify-compile-warnings-rml-1.patch --]
[-- Type: text/x-patch, Size: 1320 bytes --]

Fix a couple misc. compile warnings in inotify.c
Signed-Off-By: Robert Love <rml@novell.com>

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

--- linux-inotify/drivers/char/inotify.c	2004-09-21 00:58:03.912389824 -0400
+++ linux/drivers/char/inotify.c	2004-09-21 01:24:07.042757888 -0400
@@ -45,7 +45,7 @@
 #define MAX_INOTIFY_QUEUED_EVENTS 256 /* Only the first Z events will be queued */
 #define __BITMASK_SIZE (MAX_INOTIFY_DEV_WATCHERS / 8)
 
-#define INOTIFY_DEV_TIMER_TIME jiffies + (HZ/4)
+#define INOTIFY_DEV_TIMER_TIME	(jiffies + (HZ/4))
 
 static atomic_t watcher_count; // < MAX_INOTIFY_DEVS
 
@@ -668,7 +668,7 @@
 
 	file->private_data = dev;
 
-	dev->timer.data = dev;
+	dev->timer.data = (unsigned long) dev;
 	dev->timer.function = inotify_dev_timer;
 	dev->timer.expires = INOTIFY_DEV_TIMER_TIME;
 
@@ -745,8 +745,10 @@
 	 * watching, we just update the mask and return 0
 	 */
 	if (inotify_dev_is_watching_inode (dev, inode)) {
-		iprintk(INOTIFY_DEBUG_ERRORS, "adjusting event mask for inode %p\n", inode);
-		struct inotify_watcher *owatcher; // the old watcher
+		struct inotify_watcher *owatcher;	/* the old watcher */
+
+		iprintk(INOTIFY_DEBUG_ERRORS,
+				"adjusting event mask for inode %p\n", inode);
 
 		owatcher = inode_find_dev (inode, dev);
 

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

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

On Sun, 2004-09-19 at 23:56 -0400, John McCutchan wrote:

> I would appreciate design review, code review and testing.

Hello sir!

kmalloc() can fail, so check the return value and handle appropriately.

Patch is against inotify 0.9.2 plus the last patches I sent.

Thanks,

	Robert Love


[-- Attachment #2: inotify-check-kmalloc-rml-1.patch --]
[-- Type: text/x-patch, Size: 1273 bytes --]

kmalloc() can fail. Check the return value.

Signed-Off-By: Robert Love <rml@novell.com>

 drivers/char/inotify.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

--- linux-inotify/drivers/char/inotify.c	2004-09-21 01:30:09.160707592 -0400
+++ linux/drivers/char/inotify.c	2004-09-21 01:40:47.777623064 -0400
@@ -551,11 +551,15 @@
 	int events;
 	int event_count;
 
-	eventbuf = kmalloc(sizeof(struct inotify_event) * MAX_EVENTS_AT_ONCE, 
-			GFP_KERNEL);
+	out = -ENOMEM;
+	eventbuf = kmalloc(sizeof(struct inotify_event) * MAX_EVENTS_AT_ONCE,
+			   GFP_KERNEL);
+	if (!eventbuf)
+		goto out;
+
+	out = 0;
 	events = 0;
 	event_count = 0;
-	out = 0;
 	err = 0;
 
 	obuf = buf;
@@ -644,7 +648,8 @@
 	}
 }
 
-static int inotify_open(struct inode *inode, struct file *file) {
+static int inotify_open(struct inode *inode, struct file *file)
+{
 	struct inotify_device *dev;
 
 	if (atomic_read(&watcher_count) == MAX_INOTIFY_DEVS)
@@ -653,7 +658,11 @@
 	atomic_inc(&watcher_count);
 
 	dev = kmalloc(sizeof(struct inotify_device), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
 	dev->bitmask = kmalloc(__BITMASK_SIZE, GFP_KERNEL);
+	if (!dev->bitmask)
+		return -ENOMEM;
 	memset(dev->bitmask, 0, __BITMASK_SIZE);
 
 	INIT_LIST_HEAD(&dev->events);

Re: [RFC][PATCH] inotify 0.9.2

[Edgar Toernig]

Robert Love wrote:
>
> +/*
> + * struct inotify_event - structure read from the inotify device for each event
> + *
> + * When you are watching a directory, you will receive the filename for events
> + * such as IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, and so on ...
> + *
> + * Note: When reading from the device you must provide a buffer that is a
> + * multiple of sizeof(struct inotify_event)
> + */
>  struct inotify_event {
>  	int wd;
>  	int mask;
> -	char filename[256];
> +	char filename[PATH_MAX];
>  };

You really want to shove >4kB per event to userspace???

Ciao, ET.

Re: [RFC][PATCH] inotify 0.9.2

[Chris Friesen]

Edgar Toernig wrote:
> Robert Love wrote:
> 
>> struct inotify_event {
>> 	int wd;
>> 	int mask;
>>-	char filename[256];
>>+	char filename[PATH_MAX];
>> };
> 
> 
> You really want to shove >4kB per event to userspace???

Ouch.

Maybe make it variable-size?  On average it would likely be shorter.

struct inotify_event {
	int wd;
	int mask;
	short namelen;
	char filename[0];
};

Chris

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

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

On Tue, 2004-09-21 at 17:34 +0200, Edgar Toernig wrote:

> You really want to shove >4kB per event to userspace???

Eh, good point.  And I guess this is just the filename.

Probably best to keep it at 256, then, until we think of something
better.

Attached patch is the header cleanup minus the PATH_MAX bit.

	Robert Love


[-- Attachment #2: inotify-cleanup-header-rml-2.patch --]
[-- Type: text/x-patch, Size: 4423 bytes --]

Clean up the inotify header

Signed-Off-By: Robert Love <rml@novell.com>

 include/linux/inotify.h |   79 +++++++++++++++++++++++++++++-------------------
 1 files changed, 48 insertions(+), 31 deletions(-)

--- linux-inotify/include/linux/inotify.h	2004-09-21 00:58:03.920388608 -0400
+++ linux/include/linux/inotify.h	2004-09-21 01:15:05.831034576 -0400
@@ -9,66 +9,83 @@
 #ifndef _LINUX_INOTIFY_H
 #define _LINUX_INOTIFY_H
 
-struct inode;
-struct dentry;
-struct super_block;
+#include <linux/limits.h>
 
+/*
+ * struct inotify_event - structure read from the inotify device for each event
+ *
+ * When you are watching a directory, you will receive the filename for events
+ * such as IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, and so on ...
+ *
+ * Note: When reading from the device you must provide a buffer that is a
+ * multiple of sizeof(struct inotify_event)
+ */
 struct inotify_event {
 	int wd;
 	int mask;
-	char filename[256];
-	/* When you are watching a directory you will get the filenames
-	 * for events like IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, etc.. 
-	 */
+	char filename[256];	/* XXX: This size may be a problem */
 };
-/* When reading from the device you must provide a buffer 
- * that is a multiple of the sizeof(inotify_event)
- */
 
+/* the following are legal, implemented events */
 #define IN_ACCESS	0x00000001	/* File was accessed */
 #define IN_MODIFY	0x00000002	/* File was modified */
 #define IN_CREATE	0x00000004	/* File was created */
 #define IN_DELETE	0x00000008	/* File was deleted */
 #define IN_RENAME	0x00000010	/* File was renamed */
-#define IN_ATTRIB	0x00000020	/* File changed attributes */
-#define IN_MOVE		0x00000040	/* File was moved */
-#define IN_UNMOUNT	0x00000080	/* Device file was on, was unmounted */
+#define IN_UNMOUNT	0x00000080	/* Backing filesystem was unmounted */
 #define IN_CLOSE	0x00000100	/* File was closed */
 #define IN_OPEN		0x00000200	/* File was opened */
+
+/* the following are legal, but not yet implemented, events */
+#define IN_ATTRIB	0x00000020	/* File changed attributes */
+#define IN_MOVE		0x00000040	/* File was moved */
+
+/* special flags */
 #define IN_IGNORED	0x00000400	/* File was ignored */
 #define IN_ALL_EVENTS	0xffffffff	/* All the events */
 
-/* ioctl */
-
-/* Fill this and pass it to INOTIFY_WATCH ioctl */
+/*
+ * struct inotify_watch_request - represents a watch request
+ *
+ * Pass to the inotify device via the INOTIFY_WATCH ioctl
+ */
 struct inotify_watch_request {
-	char *dirname; // directory name
-	unsigned long mask; // event mask
+	char *dirname;		/* directory name */
+	unsigned long mask;	/* event mask */
 };
 
-#define INOTIFY_IOCTL_MAGIC 'Q'
-#define INOTIFY_IOCTL_MAXNR 4
+#define INOTIFY_IOCTL_MAGIC	'Q'
+#define INOTIFY_IOCTL_MAXNR	4
 
 #define INOTIFY_WATCH  		_IOR(INOTIFY_IOCTL_MAGIC, 1, struct inotify_watch_request)
 #define INOTIFY_IGNORE 		_IOR(INOTIFY_IOCTL_MAGIC, 2, int)
 #define INOTIFY_STATS		_IOR(INOTIFY_IOCTL_MAGIC, 3, int)
 #define INOTIFY_SETDEBUG	_IOR(INOTIFY_IOCTL_MAGIC, 4, int)
 
-#define INOTIFY_DEBUG_NONE   0x00000000
-#define INOTIFY_DEBUG_ALLOC  0x00000001
-#define INOTIFY_DEBUG_EVENTS 0x00000002
-#define INOTIFY_DEBUG_INODE  0x00000004
-#define INOTIFY_DEBUG_ERRORS 0x00000008
-#define INOTIFY_DEBUG_FILEN  0x00000010
-#define INOTIFY_DEBUG_ALL    0xffffffff
+#define INOTIFY_DEBUG_NONE	0x00000000
+#define INOTIFY_DEBUG_ALLOC	0x00000001
+#define INOTIFY_DEBUG_EVENTS	0x00000002
+#define INOTIFY_DEBUG_INODE	0x00000004
+#define INOTIFY_DEBUG_ERRORS	0x00000008
+#define INOTIFY_DEBUG_FILEN	0x00000010
+#define INOTIFY_DEBUG_ALL	0xffffffff
+
+#ifdef __KERNEL__
+
+#include <linux/dcache.h>
+#include <linux/fs.h>
 
-/* Kernel API */
 /* Adds events to all watchers on inode that are interested in mask */
-void inotify_inode_queue_event (struct inode *inode, unsigned long mask, const char *filename);
+void inotify_inode_queue_event (struct inode *inode, unsigned long mask,
+		const char *filename);
+
 /* Same as above but uses dentry's inode */
-void inotify_dentry_parent_queue_event (struct dentry *dentry, unsigned long mask, const char *filename);
+void inotify_dentry_parent_queue_event (struct dentry *dentry,
+		unsigned long mask, const char *filename);
+
 /* This will remove all watchers from all inodes on the superblock */
 void inotify_super_block_umount (struct super_block *sb);
 
-#endif
+#endif	/* __KERNEL __ */
 
+#endif	/* _LINUX_INOTIFY_H */

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

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

On Sun, 2004-09-19 at 23:56 -0400, John McCutchan wrote:

> I would appreciate design review, code review and testing.

Hey, John.

We are seeing an oops when monitoring a large number of directories.
The system keeps running, but I/O gets flaky and eventually processes
start getting stuck.

Also, the ioctl() stops returning new WD after 1024.  Thereafter, it
keeps returning the same value.

I have attached the relevant bits from the syslog.  I will debug it, but
I thought that perhaps you would immediately see the issue.

Thanks,

	Robert Love


[-- Attachment #2: inotify-oops --]
[-- Type: text/plain, Size: 14260 bytes --]

Sep 21 13:46:21 slim kernel: inotify device opened
Sep 21 13:46:21 slim kernel: inotify device released
<repeated many times>
Sep 21 14:06:05 slim kernel: inotify device opened
Sep 21 14:06:09 slim kernel: inotify device released
Sep 21 14:06:32 slim kernel: inotify device opened
Sep 21 14:06:32 slim kernel: GLOBAL INOTIFY STATS
Sep 21 14:06:32 slim kernel: watcher_count = 1
Sep 21 14:06:32 slim kernel: event_object_count = 0
Sep 21 14:06:32 slim kernel: watcher_object_count = 0
Sep 21 14:06:32 slim kernel: inode_ref_count = 0
Sep 21 14:06:32 slim kernel: sizeof(struct inotify_watcher) = 40
Sep 21 14:06:32 slim kernel: sizeof(struct inotify_device) = 68
Sep 21 14:06:32 slim kernel: sizeof(struct inotify_kernel_event) = 272
Sep 21 14:06:32 slim kernel: inotify device: e2b75900
Sep 21 14:06:32 slim kernel: inotify event_count: 0
Sep 21 14:06:32 slim kernel: inotify watch_count: 0
Sep 21 14:07:14 slim kernel: inotify device released
Sep 21 14:08:27 slim kernel: inotify device opened
Sep 21 14:08:27 slim kernel: GLOBAL INOTIFY STATS
Sep 21 14:08:27 slim kernel: watcher_count = 1
Sep 21 14:08:27 slim kernel: event_object_count = 0
Sep 21 14:08:27 slim kernel: watcher_object_count = 0
Sep 21 14:08:27 slim kernel: inode_ref_count = 0
Sep 21 14:08:27 slim kernel: sizeof(struct inotify_watcher) = 40
Sep 21 14:08:27 slim kernel: sizeof(struct inotify_device) = 68
Sep 21 14:08:27 slim kernel: sizeof(struct inotify_kernel_event) = 272
Sep 21 14:08:27 slim kernel: inotify device: f70de900
Sep 21 14:08:27 slim kernel: inotify event_count: 0
Sep 21 14:08:27 slim kernel: inotify watch_count: 0
Sep 21 14:08:41 slim kernel: inotify device released
Sep 21 14:10:57 slim kernel: inotify device opened
Sep 21 14:11:38 slim kernel: inotify device released
Sep 21 14:12:14 slim kernel: inotify device opened
<repeated many times>
Sep 21 14:31:13 slim kernel: inotify device released
Sep 21 14:32:10 slim kernel: inotify device opened
Sep 21 14:32:31 slim kernel: Unable to handle kernel paging request at virtual address 00017f2d
Sep 21 14:32:31 slim kernel:  printing eip:
Sep 21 14:32:31 slim kernel: c02095f4
Sep 21 14:32:31 slim kernel: *pde = 00000000
Sep 21 14:32:31 slim kernel: Oops: 0002 [#1]
Sep 21 14:32:31 slim kernel: CPU:    0
Sep 21 14:32:31 slim kernel: EIP:    0060:[<c02095f4>]    Tainted: P  X
Sep 21 14:32:31 slim kernel: EFLAGS: 00010282   (2.6.5-707.inotify.0-default) 
Sep 21 14:32:31 slim kernel: EIP is at inotify_dev_queue_event+0x74/0xd0
Sep 21 14:32:31 slim kernel: eax: 00017f2d   ebx: 00000200   ecx: 0000000a   edx: c18c38c0
Sep 21 14:32:31 slim kernel: esi: c19bf080   edi: f705ef80   ebp: 00000200   esp: c7e1df64
Sep 21 14:32:31 slim kernel: ds: 007b   es: 007b   ss: 0068
Sep 21 14:32:31 slim kernel: Process mono (pid: 7712, threadinfo=c7e1c000 task=f701c8d0)
Sep 21 14:32:31 slim kernel: Stack: db7bb540 d2c6d534 00000000 c02098af 00000000 00000018 c7e1df90 e62d1480 
Sep 21 14:32:32 slim kernel:        f575e000 c0155c5e 00000000 00000000 00000000 f575e000 00018800 40d354c0 
Sep 21 14:32:32 slim kernel:        c03d6c50 f701c8d0 f701ca7c 081ba728 081ba728 081ba768 c7e1c000 c0107dc9 
Sep 21 14:32:32 slim kernel: Call Trace:
Sep 21 14:32:32 slim kernel:  [<c02098af>] inotify_inode_queue_event+0x2f/0x50
Sep 21 14:32:32 slim kernel:  [<c0155c5e>] sys_open+0x9e/0xf0
Sep 21 14:32:32 slim kernel:  [<c0107dc9>] sysenter_past_esp+0x52/0x79
Sep 21 14:32:32 slim kernel: 
Sep 21 14:32:32 slim kernel: Code: 89 30 f6 05 44 8d 42 c0 02 75 21 90 5b 5e 5f c3 f6 05 44 8d 
Sep 21 14:33:08 slim kernel:  <1>inotify device released
Sep 21 14:33:13 slim kernel: Assertion failure in __journal_drop_transaction() at fs/jbd/checkpoint.c:610: "transaction->t_state == T_FINISHED"
Sep 21 14:33:13 slim kernel: ------------[ cut here ]------------
Sep 21 14:33:13 slim kernel: kernel BUG at fs/jbd/checkpoint.c:610!
Sep 21 14:33:13 slim kernel: invalid operand: 0000 [#2]
Sep 21 14:33:13 slim kernel: CPU:    0
Sep 21 14:33:13 slim kernel: EIP:    0060:[<fa83dee6>]    Tainted: P  X
Sep 21 14:33:13 slim kernel: EFLAGS: 00010286   (2.6.5-707.inotify.0-default) 
Sep 21 14:33:13 slim kernel: EIP is at __journal_drop_transaction+0x46/0x330 [jbd]
Sep 21 14:33:13 slim kernel: eax: 00000076   ebx: f705ef80   ecx: 00000002   edx: f731ff38
Sep 21 14:33:13 slim kernel: esi: f7eb0580   edi: f5f3d8a0   ebp: f705ef80   esp: f7a2bdac
Sep 21 14:33:13 slim kernel: ds: 007b   es: 007b   ss: 0068
Sep 21 14:33:13 slim kernel: Process kjournald (pid: 874, threadinfo=f7a2a000 task=f7a2d7c0)
Sep 21 14:33:13 slim kernel: Stack: fa842a94 fa8413f3 fa841b28 00000262 fa842afc f705ef80 f7eb0580 fa83e20c 
Sep 21 14:33:13 slim kernel:        d0410250 f5f3d8a0 fa83e26b f5f3d8a0 fa83e2c3 00000003 e5874f80 f7567500 
Sep 21 14:33:13 slim kernel:        f7eb0580 e2b75c00 00000000 00000000 fa83c239 f7a2a000 f7a2a000 00000000 
Sep 21 14:33:13 slim kernel: Call Trace:
Sep 21 14:33:13 slim kernel:  [<fa83e20c>] __journal_remove_checkpoint+0x3c/0x70 [jbd]
Sep 21 14:33:13 slim kernel:  [<fa83e26b>] __try_to_free_cp_buf+0x2b/0x40 [jbd]
Sep 21 14:33:13 slim kernel:  [<fa83e2c3>] __journal_clean_checkpoint_list+0x43/0x60 [jbd]
Sep 21 14:33:13 slim kernel:  [<fa83c239>] journal_commit_transaction+0x1b9/0x1310 [jbd]
Sep 21 14:33:13 slim kernel:  [<c011f1c0>] autoremove_wake_function+0x0/0x30
Sep 21 14:33:13 slim kernel:  [<c011b9fa>] recalc_task_prio+0x1da/0x470
Sep 21 14:33:13 slim kernel:  [<c011ca76>] schedule+0x1f6/0x6d0
Sep 21 14:33:13 slim kernel:  [<fa840f03>] kjournald+0xc3/0x310 [jbd]
Sep 21 14:33:13 slim kernel:  [<c0121dfa>] do_exit+0x86a/0xa50
Sep 21 14:33:13 slim kernel:  [<c011f1c0>] autoremove_wake_function+0x0/0x30
Sep 21 14:33:13 slim kernel:  [<c011f1c0>] autoremove_wake_function+0x0/0x30
Sep 21 14:33:13 slim kernel:  [<c0107d16>] ret_from_fork+0x6/0x20
Sep 21 14:33:13 slim kernel:  [<fa841150>] commit_timeout+0x0/0x10 [jbd]
Sep 21 14:33:13 slim kernel:  [<fa840e40>] kjournald+0x0/0x310 [jbd]
Sep 21 14:33:13 slim kernel:  [<c0106005>] kernel_thread_helper+0x5/0x10
Sep 21 14:33:13 slim kernel: 
Sep 21 14:33:13 slim kernel: Code: 0f 0b 62 02 28 1b 84 fa 83 c4 14 8b 4b 1c 85 c9 0f 85 24 02 


----------------------------------------------------------


Sep 21 14:39:00 slim kernel: inotify device opened
Sep 21 14:41:04 slim kernel: inotify device released
Sep 21 14:41:11 slim kernel: inotify device opened
Sep 21 14:41:24 slim kernel: Unable to handle kernel paging request at virtual address 00200200
Sep 21 14:41:24 slim kernel:  printing eip:
Sep 21 14:41:24 slim kernel: c02095f4
Sep 21 14:41:24 slim kernel: *pde = 00000000
Sep 21 14:41:24 slim kernel: Oops: 0002 [#1]
Sep 21 14:41:24 slim kernel: CPU:    0
Sep 21 14:41:24 slim kernel: EIP:    0060:[<c02095f4>]    Tainted: P  X
Sep 21 14:41:24 slim kernel: EFLAGS: 00010282   (2.6.5-707.inotify.0-default) 
Sep 21 14:41:24 slim kernel: EIP is at inotify_dev_queue_event+0x74/0xd0
Sep 21 14:41:24 slim kernel: eax: 00200200   ebx: 00000200   ecx: 00000000   edx: 00000020
Sep 21 14:41:24 slim kernel: esi: e4387080   edi: eefa2600   ebp: 00000200   esp: e4b97f58
Sep 21 14:41:24 slim kernel: ds: 007b   es: 007b   ss: 0068
Sep 21 14:41:24 slim kernel: Process mono (pid: 5987, threadinfo=e4b96000 task=e66448e0)
Sep 21 14:41:24 slim kernel: Stack: df826608 e65dcb34 e1691758 c02098af e1691758 e8632114 e16916dc e43c6080 
Sep 21 14:41:24 slim kernel:        f6a51000 c020a2ae 00000018 e4b97f90 c0155c6e 00000000 00000000 00000000 
Sep 21 14:41:24 slim kernel:        f6a51000 00018800 000002e0 c03d70c8 e66448e0 e6644a8c 081c5348 081c5348 
Sep 21 14:41:24 slim kernel: Call Trace:
Sep 21 14:41:24 slim kernel:  [<c02098af>] inotify_inode_queue_event+0x2f/0x50
Sep 21 14:41:24 slim kernel:  [<c020a2ae>] inotify_dentry_parent_queue_event+0x1e/0x40
Sep 21 14:41:24 slim kernel:  [<c0155c6e>] sys_open+0xae/0xf0
Sep 21 14:41:24 slim kernel:  [<c0107dc9>] sysenter_past_esp+0x52/0x79
Sep 21 14:41:24 slim kernel: 
Sep 21 14:41:24 slim kernel: Code: 89 30 f6 05 44 8d 42 c0 02 75 21 90 5b 5e 5f c3 f6 05 44 8d 
Sep 21 14:41:44 slim kernel:  <1>inotify device released
Sep 21 14:42:09 slim kernel: inotify device opened
Sep 21 14:42:09 slim kernel: Unable to handle kernel paging request at virtual address 00029507
Sep 21 14:42:09 slim kernel:  printing eip:
Sep 21 14:42:09 slim kernel: c02095f4
Sep 21 14:42:09 slim kernel: *pde = 00000000
Sep 21 14:42:09 slim kernel: Oops: 0002 [#2]
Sep 21 14:42:09 slim kernel: CPU:    0
Sep 21 14:42:09 slim kernel: EIP:    0060:[<c02095f4>]    Tainted: P  X
Sep 21 14:42:09 slim kernel: EFLAGS: 00010286   (2.6.5-707.inotify.0-default) 
Sep 21 14:42:09 slim kernel: EIP is at inotify_dev_queue_event+0x74/0xd0
Sep 21 14:42:09 slim kernel: eax: 00029507   ebx: 00000200   ecx: 0000000f   edx: c18c38c0
Sep 21 14:42:09 slim kernel: esi: e4387e50   edi: eefa2600   ebp: 00000200   esp: e82bdf64
Sep 21 14:42:09 slim kernel: ds: 007b   es: 007b   ss: 0068
Sep 21 14:42:09 slim kernel: Process mono (pid: 6081, threadinfo=e82bc000 task=e66448e0)
Sep 21 14:42:09 slim kernel: Stack: df826518 f37c0b34 00000000 c02098af 00000000 00000018 e82bdf90 df3fcc80 
Sep 21 14:42:09 slim kernel:        f4eb6000 c0155c5e 00000000 00000000 00000000 f4eb6000 00018800 400172d0 
Sep 21 14:42:09 slim kernel:        c03d70c8 4022e578 00000004 081bae50 081bae50 081b91e0 e82bc000 c0107dc9 
Sep 21 14:42:09 slim kernel: Call Trace:
Sep 21 14:42:09 slim kernel:  [<c02098af>] inotify_inode_queue_event+0x2f/0x50
Sep 21 14:42:09 slim kernel:  [<c0155c5e>] sys_open+0x9e/0xf0
Sep 21 14:42:09 slim kernel:  [<c0107dc9>] sysenter_past_esp+0x52/0x79
Sep 21 14:42:09 slim kernel: 
Sep 21 14:42:09 slim kernel: Code: 89 30 f6 05 44 8d 42 c0 02 75 21 90 5b 5e 5f c3 f6 05 44 8d 
Sep 21 14:42:18 slim trow: spamd starting 
Sep 21 14:42:21 slim kernel:  <1>Unable to handle kernel paging request at virtual address 00017f76
Sep 21 14:42:21 slim kernel:  printing eip:
Sep 21 14:42:21 slim kernel: c02095f4
Sep 21 14:42:21 slim kernel: *pde = 00000000
Sep 21 14:42:21 slim kernel: Oops: 0002 [#3]
Sep 21 14:42:21 slim kernel: CPU:    0
Sep 21 14:42:21 slim kernel: EIP:    0060:[<c02095f4>]    Tainted: P  X
Sep 21 14:42:21 slim kernel: EFLAGS: 00010286   (2.6.5-707.inotify.0-default) 
Sep 21 14:42:21 slim kernel: EIP is at inotify_dev_queue_event+0x74/0xd0
Sep 21 14:42:21 slim kernel: eax: 00017f76   ebx: 00000002   ecx: 00000000   edx: c18c38c0
Sep 21 14:42:21 slim kernel: esi: df0c6e50   edi: eefa2600   ebp: 00000002   esp: eb33bf30
Sep 21 14:42:21 slim kernel: ds: 007b   es: 007b   ss: 0068
Sep 21 14:42:21 slim kernel: Process evolution-2.0 (pid: 5467, threadinfo=eb33a000 task=eb32f410)
Sep 21 14:42:21 slim kernel: Stack: df826518 f37c0b34 f3c9d7ec c02098af f3c9d7ec f3bc4364 f3c9d770 c1a993a4 
Sep 21 14:42:21 slim kernel:        c1a993a4 c020a2ae 0000001d c1a99380 c0158ab4 41553c98 00000002 eb33bf90 
Sep 21 14:42:21 slim kernel:        c1a99380 0000001d c0158ce1 c1a993a4 00000001 fffffff7 c01555e3 f5fcc180 
Sep 21 14:42:21 slim kernel: Call Trace:
Sep 21 14:42:21 slim kernel:  [<c02098af>] inotify_inode_queue_event+0x2f/0x50
Sep 21 14:42:21 slim kernel:  [<c020a2ae>] inotify_dentry_parent_queue_event+0x1e/0x40
Sep 21 14:42:21 slim kernel:  [<c0158ab4>] vfs_write+0xc4/0x120
Sep 21 14:42:21 slim kernel:  [<c0158ce1>] sys_write+0x81/0xd0
Sep 21 14:42:21 slim kernel:  [<c01555e3>] filp_close+0x43/0x70
Sep 21 14:42:21 slim kernel:  [<c0107dc9>] sysenter_past_esp+0x52/0x79
Sep 21 14:42:21 slim kernel: 
Sep 21 14:42:21 slim kernel: Code: 89 30 f6 05 44 8d 42 c0 02 75 21 90 5b 5e 5f c3 f6 05 44 8d 
Sep 21 14:42:30 slim kernel:  <1>inotify device released
Sep 21 14:42:42 slim kernel: Assertion failure in __journal_drop_transaction() at fs/jbd/checkpoint.c:610: "transaction->t_state == T_FINISHED"
Sep 21 14:42:42 slim kernel: ------------[ cut here ]------------
Sep 21 14:42:42 slim kernel: kernel BUG at fs/jbd/checkpoint.c:610!
Sep 21 14:42:42 slim kernel: invalid operand: 0000 [#4]
Sep 21 14:42:42 slim kernel: CPU:    0
Sep 21 14:42:42 slim kernel: EIP:    0060:[<fa83dee6>]    Tainted: P  X
Sep 21 14:42:42 slim kernel: EFLAGS: 00010286   (2.6.5-707.inotify.0-default) 
Sep 21 14:42:42 slim kernel: EIP is at __journal_drop_transaction+0x46/0x330 [jbd]
Sep 21 14:42:42 slim kernel: eax: 00000076   ebx: eefa2600   ecx: 00000002   edx: f6371f38
Sep 21 14:42:42 slim kernel: esi: f7eb0580   edi: ee3b4360   ebp: eefa2600   esp: c1a55dac
Sep 21 14:42:42 slim kernel: ds: 007b   es: 007b   ss: 0068
Sep 21 14:42:42 slim kernel: Process kjournald (pid: 874, threadinfo=c1a54000 task=c1a5c670)
Sep 21 14:42:42 slim kernel: Stack: fa842a94 fa8413f3 fa841b28 00000262 fa842afc eefa2600 f7eb0580 fa83e20c 
Sep 21 14:42:42 slim kernel:        ded9e284 ee3b4360 fa83e26b ee3b4360 fa83e2c3 00000009 e063f280 f5d9b780 
Sep 21 14:42:42 slim kernel:        f7eb0580 e1aa3480 00000000 00000000 fa83c239 c1a54000 c1a54000 00000000 
Sep 21 14:42:42 slim kernel: Call Trace:
Sep 21 14:42:42 slim kernel:  [<fa83e20c>] __journal_remove_checkpoint+0x3c/0x70 [jbd]
Sep 21 14:42:42 slim kernel:  [<fa83e26b>] __try_to_free_cp_buf+0x2b/0x40 [jbd]
Sep 21 14:42:42 slim kernel:  [<fa83e2c3>] __journal_clean_checkpoint_list+0x43/0x60 [jbd]
Sep 21 14:42:42 slim kernel:  [<fa83c239>] journal_commit_transaction+0x1b9/0x1310 [jbd]
Sep 21 14:42:42 slim kernel:  [<c011f1c0>] autoremove_wake_function+0x0/0x30
Sep 21 14:42:42 slim kernel:  [<c011f1c0>] autoremove_wake_function+0x0/0x30
Sep 21 14:42:42 slim kernel:  [<c011b9fa>] recalc_task_prio+0x1da/0x470
Sep 21 14:42:42 slim kernel:  [<c011ca76>] schedule+0x1f6/0x6d0
Sep 21 14:42:42 slim kernel:  [<fa840f03>] kjournald+0xc3/0x310 [jbd]
Sep 21 14:42:42 slim kernel:  [<c0121dfa>] do_exit+0x86a/0xa50
Sep 21 14:42:42 slim kernel:  [<c011f1c0>] autoremove_wake_function+0x0/0x30
Sep 21 14:42:42 slim kernel:  [<c011f1c0>] autoremove_wake_function+0x0/0x30
Sep 21 14:42:42 slim kernel:  [<c0107d16>] ret_from_fork+0x6/0x20
Sep 21 14:42:42 slim kernel:  [<fa841150>] commit_timeout+0x0/0x10 [jbd]
Sep 21 14:42:42 slim kernel:  [<fa840e40>] kjournald+0x0/0x310 [jbd]
Sep 21 14:42:42 slim kernel:  [<c0106005>] kernel_thread_helper+0x5/0x10
Sep 21 14:42:42 slim kernel: 
Sep 21 14:42:42 slim kernel: Code: 0f 0b 62 02 28 1b 84 fa 83 c4 14 8b 4b 1c 85 c9 0f 85 24 02 
Sep 21 14:44:22 slim syslogd 1.4.1: restart.

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

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

On Tue, 2004-09-21 at 12:04 -0400, Robert Love wrote:

> Hey, John.
> 
> We are seeing an oops when monitoring a large number of directories.
> The system keeps running, but I/O gets flaky and eventually processes
> start getting stuck.
> 
> Also, the ioctl() stops returning new WD after 1024.  Thereafter, it
> keeps returning the same value.
> 
> I have attached the relevant bits from the syslog.  I will debug it, but
> I thought that perhaps you would immediately see the issue.

OK.  I fixed the problem with ioctl() failing after 1024 WD's.  This may
also fix the oopses.  Still checking on that.

The problem was that we were passing the size of dev->bitmask in _bytes_
to find_first_zero_bit().  But find_first_zero_bit()'s second parameter
is the size in _bits_.

I then went ahead and just made dev->bitmask an array, since we know the
size at compile time.

Comments?

Best,

	Robert Love


[-- Attachment #2: inotify-fix-wd-rml-1.patch --]
[-- Type: text/x-patch, Size: 1519 bytes --]

Fix problem with ioctl() failing after 1024 WD's, by giving ffz bits not bytes.
Signed-Off-By: Robert Love <rml@novell.com>

 drivers/char/inotify.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

--- linux-inotify-rml/drivers/char/inotify.c	2004-09-20 18:02:32.886258448 -0400
+++ linux/drivers/char/inotify.c	2004-09-21 14:51:11.433908024 -0400
@@ -43,7 +43,6 @@
 #define MAX_INOTIFY_DEVS 8 /* We only support X watchers */
 #define MAX_INOTIFY_DEV_WATCHERS 8192 /* A dev can only have Y watchers */
 #define MAX_INOTIFY_QUEUED_EVENTS 256 /* Only the first Z events will be queued */
-#define __BITMASK_SIZE (MAX_INOTIFY_DEV_WATCHERS / 8)
 
 #define INOTIFY_DEV_TIMER_TIME jiffies + (HZ/4)
 
@@ -71,7 +70,7 @@
 	struct timer_list	timer;
 	char			read_state;
 	spinlock_t		lock;
-	void *			bitmask;
+	unsigned long		bitmask[MAX_INOTIFY_DEV_WATCHERS / BITS_PER_LONG];
 };
 #define inotify_device_event_list(pos) list_entry((pos), struct inotify_event, list)
 
@@ -239,7 +238,7 @@
 
 	dev->watcher_count++;
 
-	wd = find_first_zero_bit (dev->bitmask, __BITMASK_SIZE);
+	wd = find_first_zero_bit (dev->bitmask, MAX_INOTIFY_DEV_WATCHERS);
 
 	set_bit (wd, dev->bitmask);
 
@@ -653,8 +652,7 @@
 	atomic_inc(&watcher_count);
 
 	dev = kmalloc(sizeof(struct inotify_device), GFP_KERNEL);
-	dev->bitmask = kmalloc(__BITMASK_SIZE, GFP_KERNEL);
-	memset(dev->bitmask, 0, __BITMASK_SIZE);
+	memset(dev->bitmask, 0, MAX_INOTIFY_DEV_WATCHERS);
 
 	INIT_LIST_HEAD(&dev->events);
 	INIT_LIST_HEAD(&dev->watchers);

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

On Tue, 2004-09-21 at 14:56 -0400, Robert Love wrote:

> I then went ahead and just made dev->bitmask an array, since we know the
> size at compile time.

Forgot to remove the kfree() there, which I did not hit in testing until
now.

This patch applies on top of the previous.

	Robert Love


Signed-Off-By: Robert "Cheese Taste Good" Love <rml@novell.com>

 drivers/char/inotify.c |    1 -
 1 files changed, 1 deletion(-)

--- linux-inotify-rml/drivers/char/inotify.c.orig	2004-09-21 16:50:00.643770936 -0400
+++ linux-inotify-rml/drivers/char/inotify.c	2004-09-21 16:50:45.118009824 -0400
@@ -711,7 +711,6 @@
 
 		inotify_release_all_events(dev);
 
-		kfree (dev->bitmask);
 		kfree (dev);
 
 	}

Re: [RFC][PATCH] inotify 0.9.2

[John McCutchan]

On Tue, 2004-09-21 at 11:43, Chris Friesen wrote:
> Edgar Toernig wrote:
> > Robert Love wrote:
> > 
> >> struct inotify_event {
> >> 	int wd;
> >> 	int mask;
> >>-	char filename[256];
> >>+	char filename[PATH_MAX];
> >> };
> > 
> > 
> > You really want to shove >4kB per event to userspace???
> 
> Ouch.
> 
> Maybe make it variable-size?  On average it would likely be shorter.
> 
> struct inotify_event {
> 	int wd;
> 	int mask;
> 	short namelen;
> 	char filename[0];
> };

This makes reading events from inotify a pain, first you need to read up
to namelen, then read namelen more bytes. 

John

Re: [RFC][PATCH] inotify 0.9.2

[John McCutchan]

On Tue, 2004-09-21 at 14:56, Robert Love wrote:
> On Tue, 2004-09-21 at 12:04 -0400, Robert Love wrote:
> 
> > Hey, John.
> > 
> > We are seeing an oops when monitoring a large number of directories.
> > The system keeps running, but I/O gets flaky and eventually processes
> > start getting stuck.
> > 
> > Also, the ioctl() stops returning new WD after 1024.  Thereafter, it
> > keeps returning the same value.
> > 
> > I have attached the relevant bits from the syslog.  I will debug it, but
> > I thought that perhaps you would immediately see the issue.
> 
> OK.  I fixed the problem with ioctl() failing after 1024 WD's.  This may
> also fix the oopses.  Still checking on that.
> 

I hope it fixes the oopses, I have only just started looking at the oops
you sent me, and nothing jumped out at me.

> The problem was that we were passing the size of dev->bitmask in _bytes_
> to find_first_zero_bit().  But find_first_zero_bit()'s second parameter
> is the size in _bits_.
>

Good to know, I wasn't sure when I wrote this code.

> I then went ahead and just made dev->bitmask an array, since we know the
> size at compile time.
> 
> Comments?

Sounds good.

John

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

On Tue, 2004-09-21 at 22:32 -0400, John McCutchan wrote:

> I hope it fixes the oopses, I have only just started looking at the oops
> you sent me, and nothing jumped out at me.

It seems to have fixed the oops.

We are seeing nothing but perfection with the updated patch that I
posted. ;-)

	Robert Love

Re: [RFC][PATCH] inotify 0.9.2

[Ray Lee]

On Tue, 2004-09-21 at 19:27, John McCutchan wrote:
> On Tue, 2004-09-21 at 11:43, Chris Friesen wrote:
> > Edgar Toernig wrote:
> > > Robert Love wrote:
> > > 
> > >> struct inotify_event {
> > >> 	int wd;
> > >> 	int mask;
> > >>-	char filename[256];
> > >>+	char filename[PATH_MAX];
> > >> };
> > > 
> > > 
> > > You really want to shove >4kB per event to userspace???
> > 
> > Ouch.
> > 
> > Maybe make it variable-size?  On average it would likely be shorter.
> > 
> > struct inotify_event {
> > 	int wd;
> > 	int mask;
> > 	short namelen;
> > 	char filename[0];
> > };
> 
> This makes reading events from inotify a pain, first you need to read up
> to namelen, then read namelen more bytes. 
> 

Not the case. A mildly smarter userspace program would merely read
everything outstanding (or everything up to a fixed buffer length), and
then unserialize the events based on the boundaries it can figure out
from the first portion of the structure.

This is a way common technique.

At least in code I write, anyway :-).

Ray

Re: [RFC][PATCH] inotify 0.9.2

[John McCutchan]

On Wed, 2004-09-22 at 21:46, Ray Lee wrote:
> On Tue, 2004-09-21 at 19:27, John McCutchan wrote:
> > On Tue, 2004-09-21 at 11:43, Chris Friesen wrote:
> > > Edgar Toernig wrote:
> > > > Robert Love wrote:
> > > > 
> > > >> struct inotify_event {
> > > >> 	int wd;
> > > >> 	int mask;
> > > >>-	char filename[256];
> > > >>+	char filename[PATH_MAX];
> > > >> };
> > > > 
> > > > 
> > > > You really want to shove >4kB per event to userspace???
> > > 
> > > Ouch.
> > > 
> > > Maybe make it variable-size?  On average it would likely be shorter.
> > > 
> > > struct inotify_event {
> > > 	int wd;
> > > 	int mask;
> > > 	short namelen;
> > > 	char filename[0];
> > > };
> > 
> > This makes reading events from inotify a pain, first you need to read up
> > to namelen, then read namelen more bytes. 
> > 
> 
> Not the case. A mildly smarter userspace program would merely read
> everything outstanding (or everything up to a fixed buffer length), and
> then unserialize the events based on the boundaries it can figure out
> from the first portion of the structure.
> 
> This is a way common technique.
> 
> At least in code I write, anyway :-).

Of course this is possible, but the inotify kernel driver only allows userspace 
program to read in event sized chunks (So that the event queue
handling is kept simple)

John

Re: [RFC][PATCH] inotify 0.9.2

[Ray Lee]

Okay, just skimmed the latest patch to try to make sure I'm not talking
crazy talk. No guarantees, though.

On Wed, 2004-09-22 at 20:42, John McCutchan wrote: 
> the inotify kernel driver only allows userspace 
> program to read in event sized chunks (So that the event queue
> handling is kept simple)

It's not much more code.

Instead of calculating events as:

+static ssize_t inotify_read(struct file *file, __user char *buf,
+			   size_t count, loff_t *pos) {
[...]
+	int events;
[...]
+	/* We only hand out full inotify events */
+	if (count < sizeof(struct inotify_event)) {
+		out = -EINVAL;
+		goto out;
+	}
+
+	events = count / sizeof(struct inotify_event);

...just keep track of the actual byte count left in buf, and continue
copying until the next event would overflow buf. Require userspace to
provide a buffer at least NAME_MAX + sizeof(struct inotify_event) [*]
where the last field in the struct is declared as filename[0], which
will guarantee forward progress in passing events.

	[*] Here's one of those things that makes me think that I'm
	talking out my tush. The comments claim that only the filename
	will be returned to userspace, but later on another comment says
	that the size might technically fly up to PATH_MAX. Wassup?

Events still arrive at userspace in logical chunks; all is good.

Perhaps I'm missing something. Always a possibility, that.

BTW:
<pedantic>
+	unsigned long		bitmask[MAX_INOTIFY_DEV_WATCHERS/BITS_PER_LONG];

would be more correct if written

  unsigned long bitmask[(MAX_INOTIFY_DEV_WATCHERS + BITS_PER_LONG - 1) / BITS_PER_LONG];

</pedantic>

BTW #2: 'mask' is variously declared as an unsigned long and other times
as an int. Granted, the two base declarations seem to live in different
structs, but I can't figure out when a mask-like thing would want to be
signed. Please consider either changing the name or, more likely,
changing all usages to unsigned. My single linear reading through the
patch hasn't quite clarified the usage to me.

Ray

P.s. Have I mentioned that I like the inotify idea a heck of a lot
better than dnotify? Ghu save us from people who think signals are a
wonderful way to communicate complex information.

Re: [RFC][PATCH] inotify 0.9.2

[Robert Love]

On Wed, 2004-09-22 at 21:52 -0700, Ray Lee wrote:

> 	[*] Here's one of those things that makes me think that I'm
> 	talking out my tush. The comments claim that only the filename
> 	will be returned to userspace, but later on another comment says
> 	that the size might technically fly up to PATH_MAX. Wassup?

Technically speaking, a single filename can be as large as PATH_MAX-1.
The comment is just a warning, though, to explain the dreary theoretical
side of the world.  Pragmatism demands that we just use
INOTIFY_FILENAME_MAX, which is a more reasonable 256.

> BTW:
> <pedantic>
> +	unsigned long		bitmask[MAX_INOTIFY_DEV_WATCHERS/BITS_PER_LONG];
> 
> would be more correct if written
> 
>   unsigned long bitmask[(MAX_INOTIFY_DEV_WATCHERS + BITS_PER_LONG - 1) / BITS_PER_LONG];
> 
> </pedantic>

Indeed!  Although we define MAX_INOTIFY_DEV_WATCHERS right above and it
is a power of two.

> BTW #2: 'mask' is variously declared as an unsigned long and other times
> as an int. Granted, the two base declarations seem to live in different
> structs, but I can't figure out when a mask-like thing would want to be
> signed. Please consider either changing the name or, more likely,
> changing all usages to unsigned. My single linear reading through the
> patch hasn't quite clarified the usage to me.

Probably should just be an 'unsigned int' everywhere.

But there are a few variables that have the same name in various
structures.  That confuses me to no end, but I am jumpy like that.

> P.s. Have I mentioned that I like the inotify idea a heck of a lot
> better than dnotify? Ghu save us from people who think signals are a
> wonderful way to communicate complex information.

Oh, dude, inotify is a godsend.

	Robert Love

Re: [RFC][PATCH] inotify 0.9.2

[Ray Lee]

I still might be talking crazy talk, but...

On Wed, 2004-09-22 at 22:10, Robert Love wrote:
> The comment is just a warning, though, to explain the dreary theoretical
> side of the world.  Pragmatism demands that we just use
> INOTIFY_FILENAME_MAX, which is a more reasonable 256.

Why bother being pragmatic when we can be correct? It's not much more
code to just do it right, and allow up to PATH_MAX filenames to be
passed to userspace.

As an alternate argument, an `ls -1 | wc` on a randomly picked directory
of my filesystem reveals an average filename length of just under 11
characters. We can save some memory (and a lot of syscalls!) by packing
in events more tightly than the 256 character statically sized
rendition.

So, correct *and* efficient. Again, what am I missing here?

> > BTW:
> > <pedantic>
> > +	unsigned long		bitmask[MAX_INOTIFY_DEV_WATCHERS/BITS_PER_LONG];
> > 
> > would be more correct if written
> > 
> >   unsigned long bitmask[(MAX_INOTIFY_DEV_WATCHERS + BITS_PER_LONG - 1) / BITS_PER_LONG];
> > 
> > </pedantic>
> 
> Indeed!  Although we define MAX_INOTIFY_DEV_WATCHERS right above and it
> is a power of two.

Sure, it's a multiple of BITS_PER_LONG right *now*, but what about in
the future? It only adds compile time overhead. Regardless, it's why I
wrote my comment as 'more correct.' I won't lose any sleep over that
change not going in, but there's no reason to encourage bad coding
habits.

> Oh, dude, inotify is a godsend.

Amen. So let's get it nailed, so that this corner of the problem space
can be considered done, and we can all move on to building bigger and
better things on top of it.

Ray