Problem with inotify

Problem with inotify

[David Gómez]

Hi all ;),

I just patched 2.6.12 kernel with the inotify latest patch
(inotify-0.23-rml-2.6.12-14.patch). Inotify is working ok with the test program
provided in inotify-utils but... I can no longer mount my IDE cdrom devices
:(. Each time i try to mount a disc, the mount proccess get stuck in D state. I
don't see what's the relation between inotify and IDE devices, but if i switch
back to the unpatched 2.6.12, mounting works again.

Any ideas? Thanks

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: Problem with inotify

[Robert Love]

On Thu, 2005-06-30 at 20:18 +0200, David Gómez wrote:

> I just patched 2.6.12 kernel with the inotify latest patch
> (inotify-0.23-rml-2.6.12-14.patch). Inotify is working ok with the test program
> provided in inotify-utils but... I can no longer mount my IDE cdrom devices
> :(. Each time i try to mount a disc, the mount proccess get stuck in D state. I
> don't see what's the relation between inotify and IDE devices, but if i switch
> back to the unpatched 2.6.12, mounting works again.

Very weird.

Did everything work with an earlier inotify?

Does wchan show anything useful (ps -ewo user,pid,command,wchan)?

Does it mount successfully once, and then subsequent mounts get suck, or
does even the first mount get stuck in D?

Thanks,

	Robert Love

Re: Problem with inotify

[David Gómez]

Hi Robert,

On Jun 30 at 02:29:48, Robert Love wrote:
> > I just patched 2.6.12 kernel with the inotify latest patch
> > (inotify-0.23-rml-2.6.12-14.patch). Inotify is working ok with the test program
> > provided in inotify-utils but... I can no longer mount my IDE cdrom devices
> > :(. Each time i try to mount a disc, the mount proccess get stuck in D state. I
> > don't see what's the relation between inotify and IDE devices, but if i switch
> > back to the unpatched 2.6.12, mounting works again.
> 
> Very weird.

Indeed.

> Did everything work with an earlier inotify?

It's the first notify i've tested, so i cannot compare with previous versions.

> Does wchan show anything useful (ps -ewo user,pid,command,wchan)?

I have to reboot to test it. I'll do it and told you if i see anything
weird in wchan for the mount process.

> Does it mount successfully once, and then subsequent mounts get suck, or
> does even the first mount get stuck in D?

The first mount get stuck, and subsequent mounts too. I tried to mount my CD-writer
and after that my DVD-writer and i got two mount D processes.

Thanks for the quick answer ;)

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: Problem with inotify

[David Gómez]

Hi Robert,

On Jun 30 at 02:29:48, Robert Love wrote:
> > I just patched 2.6.12 kernel with the inotify latest patch
> > (inotify-0.23-rml-2.6.12-14.patch). Inotify is working ok with the test program
> > provided in inotify-utils but... I can no longer mount my IDE cdrom devices
> > :(. Each time i try to mount a disc, the mount proccess get stuck in D state. I
> > don't see what's the relation between inotify and IDE devices, but if i switch
> > back to the unpatched 2.6.12, mounting works again.
> 
> Very weird.
> 
> Did everything work with an earlier inotify?
> 
> Does wchan show anything useful (ps -ewo user,pid,command,wchan)?

I tested it again, wchan says "inode_wait"...


-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: Problem with inotify

[Anton Altaparmakov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1127 bytes --]

Hi,

On Thu, 30 Jun 2005, David [utf-8] Gómez wrote:
> On Jun 30 at 02:29:48, Robert Love wrote:
> > > I just patched 2.6.12 kernel with the inotify latest patch
> > > (inotify-0.23-rml-2.6.12-14.patch). Inotify is working ok with the test program
> > > provided in inotify-utils but... I can no longer mount my IDE cdrom devices
> > > :(. Each time i try to mount a disc, the mount proccess get stuck in D state. I
> > > don't see what's the relation between inotify and IDE devices, but if i switch
> > > back to the unpatched 2.6.12, mounting works again.
> > 
> > Very weird.
> > 
> > Did everything work with an earlier inotify?
> > 
> > Does wchan show anything useful (ps -ewo user,pid,command,wchan)?
> 
> I tested it again, wchan says "inode_wait"...

Do you have any ntfs volumes mounted by any chance? 

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: Problem with inotify

[David Gómez]

Hi Anton,

On Jun 30 at 09:39:47, Anton Altaparmakov wrote:
> > I tested it again, wchan says "inode_wait"...
> 
> Do you have any ntfs volumes mounted by any chance? 

No, just ext2

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: Problem with inotify

[Anton Altaparmakov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 10995 bytes --]

On Thu, 30 Jun 2005, David [utf-8] Gómez wrote:
> On Jun 30 at 09:39:47, Anton Altaparmakov wrote:
> > > I tested it again, wchan says "inode_wait"...
> > 
> > Do you have any ntfs volumes mounted by any chance? 
> 
> No, just ext2

Ok, the ntfs deadlock doesn't affect you then.

So there appears to be a change in the VFS which causes deadlocks 
associated with both mounting and umounting of filesystems, and this is 
either introduced by inotify or just made more likely (i.e. 100% of time) 
by inotify but the change is elsewhere.

Given you see inode_wait it suggests it is a simillar problem to the one I 
found in ntfs.  For a detailed description of how that deadlock comes 
about see the patch fixing it (appended to end of this email).

So what we probably are somehow getting is that say we have a lock 
(perhaps i_sem or inode_lock?) and an inode.  And we have two processes 
which do:

Process 1			Process 2
lock the inode (I_LOCK)
				take the lock
try to take the lock -> block
				wait_on_inode() -> block
Deadlock.

The question is how and where does that happen?

So far it was only triggering at umount time so it is very interesting 
that you are seeing it at mount time as well.

ps. Of course it could actually be one single process which takes I_LOCK 
and then ends up waiting on the inode later on which of course never 
happens...

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

--- This is the patch with description of the deadlock in ntfs. ---

From akpm@osdl.org Sun Jun 26 20:33:02 2005
Date: Sun, 26 Jun 2005 12:32:27 -0700
From: akpm@osdl.org
To: aia21@cam.ac.uk, aia21@cantab.net, mm-commits@vger.kernel.org
Subject: fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation.patch added to -mm tree


The patch titled

     Fix soft lockup due to NTFS: VFS part and explanation

has been added to the -mm tree.  Its filename is

     fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation.patch

Patches currently in -mm which might be from aia21@cam.ac.uk are

fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation.patch



From: Anton Altaparmakov <aia21@cam.ac.uk>

Something has changed in the core kernel such that we now get concurrent
inode write outs, one e.g via pdflush and one via sys_sync or whatever. 
This causes a nasty deadlock in ntfs.  The only clean solution
unfortunately requires a minor vfs api extension.

First the deadlock analysis:

Prerequisive knowledge: NTFS has a file $MFT (inode 0) loaded at mount
time.  The NTFS driver uses the page cache for storing the file contents as
usual.  More interestingly this file contains the table of on-disk inodes
as a sequence of MFT_RECORDs.  Thus NTFS driver accesses the on-disk inodes
by accessing the MFT_RECORDs in the page cache pages of the loaded inode
$MFT.

The situation: VFS inode X on a mounted ntfs volume is dirty.  For same
inode X, the ntfs_inode is dirty and thus corresponding on-disk inode,
which is as explained above in a dirty PAGE_CACHE_PAGE belonging to the
table of inodes ($MFT, inode 0).

What happens:

Process 1: sys_sync()/umount()/whatever...  calls __sync_single_inode() for
$MFT -> do_writepages() -> write_page for the dirty page containing the
on-disk inode X, the page is now locked -> ntfs_write_mst_block() which
clears PageUptodate() on the page to prevent anyone else getting hold of it
whilst it does the write out (this is necessary as the on-disk inode needs
"fixups" applied before the write to disk which are removed again after the
write and PageUptodate is then set again).  It then analyses the page
looking for dirty on-disk inodes and when it finds one it calls
ntfs_may_write_mft_record() to see if it is safe to write this on-disk
inode.  This then calls ilookup5() to check if the corresponding VFS inode
is in icache().  This in turn calls ifind() which waits on the inode lock
via wait_on_inode whilst holding the global inode_lock.

Process 2: pdflush results in a call to __sync_single_inode for the same
VFS inode X on the ntfs volume.  This locks the inode (I_LOCK) then calls
write-inode -> ntfs_write_inode -> map_mft_record() -> read_cache_page() of
the page (in page cache of table of inodes $MFT, inode 0) containing the
on-disk inode.  This page has PageUptodate() clear because of Process 1
(see above) so read_cache_page() blocks when tries to take the page lock
for the page so it can call ntfs_read_page().

Thus Process 1 is holding the page lock on the page containing the on-disk
inode X and it is waiting on the inode X to be unlocked in ifind() so it
can write the page out and then unlock the page.

And Process 2 is holding the inode lock on inode X and is waiting for the 
page to be unlocked so it can call ntfs_readpage() or discover that 
Process 1 set PageUptodate() again and use the page.

Thus we have a deadlock due to ifind() waiting on the inode lock.

The only sensible solution: NTFS does not care whether the VFS inode is
locked or not when it calls ilookup5() (it doesn't use the VFS inode at
all, it just uses it to find the corresponding ntfs_inode which is of
course attached to the VFS inode (both are one single struct); and it uses
the ntfs_inode which is subject to its own locking so I_LOCK is irrelevant)
hence we want a modified ilookup5_nowait() which is the same as ilookup5()
but it does not wait on the inode lock.

Without such functionality I would have to keep my own ntfs_inode cache in
the NTFS driver just so I can find ntfs_inodes independent of their VFS
inodes which would be slow, memory and cpu cycle wasting, and incredibly
stupid given the icache already exists in the VFS.

Below is a patch that does the ilookup5_nowait() implementation in 
fs/inode.c and exports it.


ilookup5_nowait.diff:

Introduce ilookup5_nowait() which is basically the same as ilookup5() but 
it does not wait on the inode's lock (i.e. it omits the wait_on_inode() 
done in ifind()).

This is needed to avoid a nasty deadlock in NTFS.

Signed-off-by: Anton Altaparmakov <aia21@cantab.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 fs/inode.c         |   45 +++++++++++++++++++++++++++++++++++++++------
 include/linux/fs.h |    3 +++
 2 files changed, 42 insertions(+), 6 deletions(-)

diff -puN fs/inode.c~fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation fs/inode.c
--- 25/fs/inode.c~fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation	2005-06-26 12:32:21.000000000 -0700
+++ 25-akpm/fs/inode.c	2005-06-26 12:32:21.000000000 -0700
@@ -757,6 +757,7 @@ EXPORT_SYMBOL(igrab);
  * @head:       the head of the list to search
  * @test:	callback used for comparisons between inodes
  * @data:	opaque data pointer to pass to @test
+ * @wait:	if true wait for the inode to be unlocked, if false do not
  *
  * ifind() searches for the inode specified by @data in the inode
  * cache. This is a generalized version of ifind_fast() for file systems where
@@ -771,7 +772,7 @@ EXPORT_SYMBOL(igrab);
  */
 static inline struct inode *ifind(struct super_block *sb,
 		struct hlist_head *head, int (*test)(struct inode *, void *),
-		void *data)
+		void *data, const int wait)
 {
 	struct inode *inode;
 
@@ -780,7 +781,8 @@ static inline struct inode *ifind(struct
 	if (inode) {
 		__iget(inode);
 		spin_unlock(&inode_lock);
-		wait_on_inode(inode);
+		if (likely(wait))
+			wait_on_inode(inode);
 		return inode;
 	}
 	spin_unlock(&inode_lock);
@@ -820,7 +822,7 @@ static inline struct inode *ifind_fast(s
 }
 
 /**
- * ilookup5 - search for an inode in the inode cache
+ * ilookup5_nowait - search for an inode in the inode cache
  * @sb:		super block of file system to search
  * @hashval:	hash value (usually inode number) to search for
  * @test:	callback used for comparisons between inodes
@@ -832,7 +834,38 @@ static inline struct inode *ifind_fast(s
  * identification of an inode.
  *
  * If the inode is in the cache, the inode is returned with an incremented
- * reference count.
+ * reference count.  Note, the inode lock is not waited upon so you have to be
+ * very careful what you do with the returned inode.  You probably should be
+ * using ilookup5() instead.
+ *
+ * Otherwise NULL is returned.
+ *
+ * Note, @test is called with the inode_lock held, so can't sleep.
+ */
+struct inode *ilookup5_nowait(struct super_block *sb, unsigned long hashval,
+		int (*test)(struct inode *, void *), void *data)
+{
+	struct hlist_head *head = inode_hashtable + hash(sb, hashval);
+
+	return ifind(sb, head, test, data, 0);
+}
+
+EXPORT_SYMBOL(ilookup5_nowait);
+
+/**
+ * ilookup5 - search for an inode in the inode cache
+ * @sb:		super block of file system to search
+ * @hashval:	hash value (usually inode number) to search for
+ * @test:	callback used for comparisons between inodes
+ * @data:	opaque data pointer to pass to @test
+ *
+ * ilookup5() uses ifind() to search for the inode specified by @hashval and
+ * @data in the inode cache. This is a generalized version of ilookup() for
+ * file systems where the inode number is not sufficient for unique
+ * identification of an inode.
+ *
+ * If the inode is in the cache, the inode lock is waited upon and the inode is
+ * returned with an incremented reference count.
  *
  * Otherwise NULL is returned.
  *
@@ -843,7 +876,7 @@ struct inode *ilookup5(struct super_bloc
 {
 	struct hlist_head *head = inode_hashtable + hash(sb, hashval);
 
-	return ifind(sb, head, test, data);
+	return ifind(sb, head, test, data, 1);
 }
 
 EXPORT_SYMBOL(ilookup5);
@@ -900,7 +933,7 @@ struct inode *iget5_locked(struct super_
 	struct hlist_head *head = inode_hashtable + hash(sb, hashval);
 	struct inode *inode;
 
-	inode = ifind(sb, head, test, data);
+	inode = ifind(sb, head, test, data, 1);
 	if (inode)
 		return inode;
 	/*
diff -puN include/linux/fs.h~fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation include/linux/fs.h
--- 25/include/linux/fs.h~fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation	2005-06-26 12:32:21.000000000 -0700
+++ 25-akpm/include/linux/fs.h	2005-06-26 12:32:21.000000000 -0700
@@ -1442,6 +1442,9 @@ extern int inode_needs_sync(struct inode
 extern void generic_delete_inode(struct inode *inode);
 extern void generic_drop_inode(struct inode *inode);
 
+extern struct inode *ilookup5_nowait(struct super_block *sb,
+		unsigned long hashval, int (*test)(struct inode *, void *),
+		void *data);
 extern struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
 		int (*test)(struct inode *, void *), void *data);
 extern struct inode *ilookup(struct super_block *sb, unsigned long ino);
_

Re: Problem with inotify

[Robert Love]

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

On Thu, 2005-06-30 at 23:35 +0100, Anton Altaparmakov wrote:

> So there appears to be a change in the VFS which causes deadlocks 
> associated with both mounting and umounting of filesystems, and this is 
> either introduced by inotify or just made more likely (i.e. 100% of time) 
> by inotify but the change is elsewhere.

The mount case is baffling.  Inotify has no interaction whatsoever with
mounting.  Unmounting, definitely--see fs/inotify.c ::
inotify_unmount_inodes().

> Given you see inode_wait it suggests it is a simillar problem to the one I 
> found in ntfs.  For a detailed description of how that deadlock comes 
> about see the patch fixing it (appended to end of this email).

This does not fix a general problem?

> So what we probably are somehow getting is that say we have a lock 
> (perhaps i_sem or inode_lock?) and an inode.  And we have two processes 
> which do:
> 
> Process 1			Process 2
> lock the inode (I_LOCK)
> 				take the lock
> try to take the lock -> block
> 				wait_on_inode() -> block
> Deadlock.
> 
> The question is how and where does that happen?

Well, in the unmount case (the more understable case, and also where the
other bug report was triggered) we grab inode_lock and i_prune_sem.  But
we can sleep (we of course drop inode_lock first), like fs/inode.c ::
invalidate_list() does.

So something else could come in and wreck havoc.  But where?

[ Attached is the latest inotify, against 2.6.13-rc1. ]

	Robert Love


[-- Attachment #2: inotify-0.23-rml-2.6.13-rc1-1.patch --]
[-- Type: text/x-patch, Size: 60372 bytes --]

inotify!

inotify is intended to correct the deficiencies of dnotify, particularly
its inability to scale and its terrible user interface:

        * dnotify requires the opening of one fd per each directory
          that you intend to watch. This quickly results in too many
          open files and pins removable media, preventing unmount.
        * dnotify is directory-based. You only learn about changes to
          directories. Sure, a change to a file in a directory affects
          the directory, but you are then forced to keep a cache of
          stat structures.
        * dnotify's interface to user-space is awful.  Signals?

inotify provides a more usable, simple, powerful solution to file change
notification:

        * inotify's interface is a device node, not SIGIO.  You open a 
          single fd to the device node, which is select()-able.
        * inotify has an event that says "the filesystem that the item
          you were watching is on was unmounted."
        * inotify can watch directories or files.

Inotify is currently used by Beagle (a desktop search infrastructure),
Gamin (a FAM replacement), and other projects.

See Documentation/filesystems/inotify.txt.

Signed-off-by: Robert Love <rml@novell.com>

 Documentation/filesystems/inotify.txt |  155 +++++
 fs/Kconfig                            |   13 
 fs/Makefile                           |    1 
 fs/attr.c                             |   33 -
 fs/compat.c                           |   12 
 fs/file_table.c                       |    3 
 fs/inode.c                            |    6 
 fs/inotify.c                          |  984 ++++++++++++++++++++++++++++++++++
 fs/namei.c                            |   30 -
 fs/nfsd/vfs.c                         |    6 
 fs/open.c                             |    3 
 fs/read_write.c                       |   15 
 fs/sysfs/file.c                       |    7 
 fs/xattr.c                            |    5 
 include/linux/fs.h                    |    6 
 include/linux/fsnotify.h              |  257 ++++++++
 include/linux/inotify.h               |  124 ++++
 include/linux/sched.h                 |    4 
 kernel/user.c                         |    4 
 19 files changed, 1604 insertions(+), 64 deletions(-)

diff -urN linux-2.6.13-rc1/Documentation/filesystems/inotify.txt linux/Documentation/filesystems/inotify.txt
--- linux-2.6.13-rc1/Documentation/filesystems/inotify.txt	1969-12-31 19:00:00.000000000 -0500
+++ linux/Documentation/filesystems/inotify.txt	2005-07-01 16:13:14.000000000 -0400
@@ -0,0 +1,155 @@
+				    inotify
+	     a powerful yet simple file change notification system
+
+
+
+Document started 15 Mar 2005 by Robert Love <rml@novell.com>
+
+(i) User Interface
+
+Inotify is controlled by a device node, /dev/inotify.  If you do not use udev,
+this device may need to be created manually.
+
+First step in using inotify is to open the device file:
+
+	int dev_fd = open ("/dev/inotify", O_RDONLY);
+
+Change events are managed by "watches".  A watch is an (object,mask) pair where
+the object is a file or directory and the mask is a bitmask of one or more
+inotify events that the application wishes to receive.  See <linux/inotify.h>
+for valid events.  A watch is referenced by a watch descriptor, or wd.
+
+Watches are added via a file descriptor to the file.
+
+Watches on a directory will return events on any files inside of the directory.
+
+Adding a watch is simple,
+
+	/* 'wd' represents the watch on fd with mask */
+	struct inotify_request req = { fd, mask };
+	int wd = ioctl (dev_fd, INOTIFY_WATCH, &req);
+
+You can add a large number of files via something like
+
+	for each file to watch {
+		struct inotify_request req;
+		int file_fd;
+
+		file_fd = open (file, O_RDONLY);
+		if (fd < 0) {
+			perror ("open");
+			break;
+		}
+
+		req.fd = file_fd;
+		req.mask = mask;
+
+		wd = ioctl (dev_fd, INOTIFY_WATCH, &req);
+
+		close (fd);
+	}
+
+You can update an existing watch in the same manner, by passing in a new mask.
+
+An existing watch is removed via the INOTIFY_IGNORE ioctl, for example
+
+	ioctl (dev_fd, INOTIFY_IGNORE, wd);
+
+Events are provided in the form of an inotify_event structure that is read(2)
+from /dev/inotify.  The filename is of dynamic length and follows the struct.
+It is of size len.  The filename is padded with null bytes to ensure proper
+alignment.  This padding is reflected in len.
+
+You can slurp multiple events by passing a large buffer, for example
+
+	size_t len = read (fd, buf, BUF_LEN);
+
+Will return as many events as are available and fit in BUF_LEN.
+
+/dev/inotify is also select()- and poll()-able.
+
+You can find the size of the current event queue via the FIONREAD ioctl.
+
+All watches are destroyed and cleaned up on close.
+
+
+(ii) Internal Kernel Implementation
+
+Each open inotify device is associated with an inotify_device structure.
+
+Each watch is associated with an inotify_watch structure.  Watches are chained
+off of each associated device and each associated inode.
+
+See fs/inotify.c for the locking and lifetime rules.
+
+
+(iii) Rationale
+
+Q: What is the design decision behind not tying the watch to the open fd of
+   the watched object?
+
+A: Watches are associated with an open inotify device, not an open file.
+   This solves the primary problem with dnotify: keeping the file open pins
+   the file and thus, worse, pins the mount.  Dnotify is therefore infeasible
+   for use on a desktop system with removable media as the media cannot be
+   unmounted.
+
+Q: What is the design decision behind using an-fd-per-device as opposed to
+   an fd-per-watch?
+
+A: An fd-per-watch quickly consumes more file descriptors than are allowed,
+   more fd's than are feasible to manage, and more fd's than are optimally
+   select()-able.  Yes, root can bump the per-process fd limit and yes, users
+   can use epoll, but requiring both is a silly and extraneous requirement.
+   A watch consumes less memory than an open file, separating the number
+   spaces is thus sensible.  The current design is what user-space developers
+   want: Users open the device, once, and add n watches, requiring but one fd
+   and no twiddling with fd limits.  Opening /dev/inotify two thousand times
+   is silly.  If we can implement user-space's preferences cleanly--and we
+   can, the idr layer makes stuff like this trivial--then we should.
+
+   There are other good arguments.  With a single fd, there is a single
+   item to block on, which is mapped to a single queue of events.  The single
+   fd returns all watch events and also any potential out-of-band data.  If
+   every fd was a separate watch,
+
+   - There would be no way to get event ordering.  Events on file foo and
+     file bar would pop poll() on both fd's, but there would be no way to tell
+     which happened first.  A single queue trivially gives you ordering.  Such
+     ordering is crucial to existing applications such as Beagle.  Imagine
+     "mv a b ; mv b a" events without ordering.
+
+   - We'd have to maintain n fd's and n internal queues with state,
+     versus just one.  It is a lot messier in the kernel.  A single, linear
+     queue is the data structure that makes sense.
+
+   - User-space developers prefer the current API.  The Beagle guys, for
+     example, love it.  Trust me, I asked.  It is not a surprise: Who'd want
+     to manage and block on 1000 fd's via select?
+
+   - You'd have to manage the fd's, as an example: Call close() when you
+     received a delete event.
+
+   - No way to get out of band data.
+
+   - 1024 is still too low.  ;-)
+
+   When you talk about designing a file change notification system that
+   scales to 1000s of directories, juggling 1000s of fd's just does not seem
+   the right interface.  It is too heavy.
+
+Q: Why a device node?
+
+A: The poor user-space interface is the second biggest problem with dnotify.
+   Signals are a terrible, terrible interface for file notification.  Or for
+   anything, for that matter.  The idea solution, from all perspectives, is a
+   file descriptor-based one that allows basic file I/O and poll/select.
+   Obtaining the fd and managing the watches could have been done either via a
+   device file or a family of new system calls.  We decided to implement a
+   device file because adding three or four new system calls that mirrored
+   open, close, and ioctl seemed silly.  A character device makes sense from
+   user-space and was easy to implement inside of the kernel.
+
+   Additionally, it _is_ possible to open /dev/inotify more than once and
+   juggle more than one queue and thus more than one associated fd.
+
diff -urN linux-2.6.13-rc1/fs/attr.c linux/fs/attr.c
--- linux-2.6.13-rc1/fs/attr.c	2005-07-01 16:10:51.000000000 -0400
+++ linux/fs/attr.c	2005-07-01 16:13:14.000000000 -0400
@@ -10,7 +10,7 @@
 #include <linux/mm.h>
 #include <linux/string.h>
 #include <linux/smp_lock.h>
-#include <linux/dnotify.h>
+#include <linux/fsnotify.h>
 #include <linux/fcntl.h>
 #include <linux/quotaops.h>
 #include <linux/security.h>
@@ -107,31 +107,8 @@
 out:
 	return error;
 }
-
 EXPORT_SYMBOL(inode_setattr);
 
-int setattr_mask(unsigned int ia_valid)
-{
-	unsigned long dn_mask = 0;
-
-	if (ia_valid & ATTR_UID)
-		dn_mask |= DN_ATTRIB;
-	if (ia_valid & ATTR_GID)
-		dn_mask |= DN_ATTRIB;
-	if (ia_valid & ATTR_SIZE)
-		dn_mask |= DN_MODIFY;
-	/* both times implies a utime(s) call */
-	if ((ia_valid & (ATTR_ATIME|ATTR_MTIME)) == (ATTR_ATIME|ATTR_MTIME))
-		dn_mask |= DN_ATTRIB;
-	else if (ia_valid & ATTR_ATIME)
-		dn_mask |= DN_ACCESS;
-	else if (ia_valid & ATTR_MTIME)
-		dn_mask |= DN_MODIFY;
-	if (ia_valid & ATTR_MODE)
-		dn_mask |= DN_ATTRIB;
-	return dn_mask;
-}
-
 int notify_change(struct dentry * dentry, struct iattr * attr)
 {
 	struct inode *inode = dentry->d_inode;
@@ -197,11 +174,9 @@
 	if (ia_valid & ATTR_SIZE)
 		up_write(&dentry->d_inode->i_alloc_sem);
 
-	if (!error) {
-		unsigned long dn_mask = setattr_mask(ia_valid);
-		if (dn_mask)
-			dnotify_parent(dentry, dn_mask);
-	}
+	if (!error)
+		fsnotify_change(dentry, ia_valid);
+
 	return error;
 }
 
diff -urN linux-2.6.13-rc1/fs/compat.c linux/fs/compat.c
--- linux-2.6.13-rc1/fs/compat.c	2005-07-01 16:10:51.000000000 -0400
+++ linux/fs/compat.c	2005-07-01 16:13:14.000000000 -0400
@@ -37,7 +37,7 @@
 #include <linux/ctype.h>
 #include <linux/module.h>
 #include <linux/dirent.h>
-#include <linux/dnotify.h>
+#include <linux/fsnotify.h>
 #include <linux/highuid.h>
 #include <linux/sunrpc/svc.h>
 #include <linux/nfsd/nfsd.h>
@@ -1307,9 +1307,13 @@
 out:
 	if (iov != iovstack)
 		kfree(iov);
-	if ((ret + (type == READ)) > 0)
-		dnotify_parent(file->f_dentry,
-				(type == READ) ? DN_ACCESS : DN_MODIFY);
+	if ((ret + (type == READ)) > 0) {
+		struct dentry *dentry = file->f_dentry;
+		if (type == READ)
+			fsnotify_access(dentry);
+		else
+			fsnotify_modify(dentry);
+	}
 	return ret;
 }
 
diff -urN linux-2.6.13-rc1/fs/file_table.c linux/fs/file_table.c
--- linux-2.6.13-rc1/fs/file_table.c	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/file_table.c	2005-07-01 16:13:14.000000000 -0400
@@ -16,6 +16,7 @@
 #include <linux/eventpoll.h>
 #include <linux/mount.h>
 #include <linux/cdev.h>
+#include <linux/fsnotify.h>
 
 /* sysctl tunables... */
 struct files_stat_struct files_stat = {
@@ -126,6 +127,8 @@
 	struct inode *inode = dentry->d_inode;
 
 	might_sleep();
+
+	fsnotify_close(file);
 	/*
 	 * The function eventpoll_release() should be the first called
 	 * in the file cleanup chain.
diff -urN linux-2.6.13-rc1/fs/inode.c linux/fs/inode.c
--- linux-2.6.13-rc1/fs/inode.c	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/inode.c	2005-07-01 16:13:14.000000000 -0400
@@ -21,6 +21,7 @@
 #include <linux/pagemap.h>
 #include <linux/cdev.h>
 #include <linux/bootmem.h>
+#include <linux/inotify.h>
 
 /*
  * This is needed for the following functions:
@@ -202,6 +203,10 @@
 	INIT_LIST_HEAD(&inode->i_data.i_mmap_nonlinear);
 	spin_lock_init(&inode->i_lock);
 	i_size_ordered_init(inode);
+#ifdef CONFIG_INOTIFY
+	INIT_LIST_HEAD(&inode->inotify_watches);
+	sema_init(&inode->inotify_sem, 1);
+#endif
 }
 
 EXPORT_SYMBOL(inode_init_once);
@@ -346,6 +351,7 @@
 
 	down(&iprune_sem);
 	spin_lock(&inode_lock);
+	inotify_unmount_inodes(&sb->s_inodes);
 	busy = invalidate_list(&sb->s_inodes, &throw_away);
 	spin_unlock(&inode_lock);
 
diff -urN linux-2.6.13-rc1/fs/inotify.c linux/fs/inotify.c
--- linux-2.6.13-rc1/fs/inotify.c	1969-12-31 19:00:00.000000000 -0500
+++ linux/fs/inotify.c	2005-07-01 16:16:29.000000000 -0400
@@ -0,0 +1,984 @@
+/*
+ * fs/inotify.c - inode-based file event notifications
+ *
+ * Authors:
+ *	John McCutchan	<ttb@tentacle.dhs.org>
+ *	Robert Love	<rml@novell.com>
+ *
+ * Copyright (C) 2005 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.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/spinlock.h>
+#include <linux/idr.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/namei.h>
+#include <linux/poll.h>
+#include <linux/device.h>
+#include <linux/miscdevice.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/writeback.h>
+#include <linux/inotify.h>
+
+#include <asm/ioctls.h>
+
+static atomic_t inotify_cookie;
+
+static kmem_cache_t *watch_cachep;
+static kmem_cache_t *event_cachep;
+
+static int max_user_devices;
+static int max_user_watches;
+static unsigned int max_queued_events;
+
+/*
+ * Lock ordering:
+ *
+ * dentry->d_lock (used to keep d_move() away from dentry->d_parent)
+ * iprune_sem (synchronize shrink_icache_memory())
+ * 	inode_lock (protects the super_block->s_inodes list)
+ * 	inode->inotify_sem (protects inode->inotify_watches and watches->i_list)
+ * 		inotify_dev->sem (protects inotify_device and watches->d_list)
+ */
+
+/*
+ * Lifetimes of the three main data structures--inotify_device, inode, and
+ * inotify_watch--are managed by reference count.
+ *
+ * inotify_device: Lifetime is from open until release.  Additional references
+ * can bump the count via get_inotify_dev() and drop the count via
+ * put_inotify_dev().
+ *
+ * inotify_watch: Lifetime is from create_watch() to destory_watch().
+ * Additional references can bump the count via get_inotify_watch() and drop
+ * the count via put_inotify_watch().
+ *
+ * inode: Pinned so long as the inode is associated with a watch, from
+ * create_watch() to put_inotify_watch().
+ */
+
+/*
+ * struct inotify_device - represents an open instance of an inotify device
+ *
+ * This structure is protected by the semaphore 'sem'.
+ */
+struct inotify_device {
+	wait_queue_head_t 	wq;		/* wait queue for i/o */
+	struct idr		idr;		/* idr mapping wd -> watch */
+	struct semaphore	sem;		/* protects this bad boy */
+	struct list_head 	events;		/* list of queued events */
+	struct list_head	watches;	/* list of watches */
+	atomic_t		count;		/* reference count */
+	struct user_struct	*user;		/* user who opened this dev */
+	unsigned int		queue_size;	/* size of the queue (bytes) */
+	unsigned int		event_count;	/* number of pending events */
+	unsigned int		max_events;	/* maximum number of events */
+};
+
+/*
+ * struct inotify_kernel_event - An inotify event, originating from a watch and
+ * queued for user-space.  A list of these is attached to each instance of the
+ * device.  In read(), this list is walked and all events that can fit in the
+ * buffer are returned.
+ *
+ * Protected by dev->sem of the device in which we are queued.
+ */
+struct inotify_kernel_event {
+	struct inotify_event	event;	/* the user-space event */
+	struct list_head        list;	/* entry in inotify_device's list */
+	char			*name;	/* filename, if any */
+};
+
+/*
+ * struct inotify_watch - represents a watch request on a specific inode
+ *
+ * d_list is protected by dev->sem of the associated watch->dev.
+ * i_list and mask are protected by inode->inotify_sem of the associated inode.
+ * dev, inode, and wd are never written to once the watch is created.
+ */
+struct inotify_watch {
+	struct list_head	d_list;	/* entry in inotify_device's list */
+	struct list_head	i_list;	/* entry in inode's list */
+	atomic_t		count;	/* reference count */
+	struct inotify_device	*dev;	/* associated device */
+	struct inode		*inode;	/* associated inode */
+	s32 			wd;	/* watch descriptor */
+	u32			mask;	/* event mask for this watch */
+};
+
+static ssize_t show_max_queued_events(struct class_device *class, char *buf)
+{
+	return sprintf(buf, "%d\n", max_queued_events);
+}
+
+static ssize_t store_max_queued_events(struct class_device *class,
+				       const char *buf, size_t count)
+{
+	unsigned int max;
+
+	if (sscanf(buf, "%u", &max) > 0 && max > 0) {
+		max_queued_events = max;
+		return strlen(buf);
+	}
+	return -EINVAL;
+}
+
+static ssize_t show_max_user_devices(struct class_device *class, char *buf)
+{
+	return sprintf(buf, "%d\n", max_user_devices);
+}
+
+static ssize_t store_max_user_devices(struct class_device *class,
+				      const char *buf, size_t count)
+{
+	int max;
+
+	if (sscanf(buf, "%d", &max) > 0 && max > 0) {
+		max_user_devices = max;
+		return strlen(buf);
+	}
+	return -EINVAL;
+}
+
+static ssize_t show_max_user_watches(struct class_device *class, char *buf)
+{
+	return sprintf(buf, "%d\n", max_user_watches);
+}
+
+static ssize_t store_max_user_watches(struct class_device *class,
+				      const char *buf, size_t count)
+{
+	int max;
+
+	if (sscanf(buf, "%d", &max) > 0 && max > 0) {
+		max_user_watches = max;
+		return strlen(buf);
+	}
+	return -EINVAL;
+}
+
+static CLASS_DEVICE_ATTR(max_queued_events, S_IRUGO | S_IWUSR,
+			 show_max_queued_events, store_max_queued_events);
+static CLASS_DEVICE_ATTR(max_user_devices, S_IRUGO | S_IWUSR,
+			 show_max_user_devices, store_max_user_devices);
+static CLASS_DEVICE_ATTR(max_user_watches, S_IRUGO | S_IWUSR,
+			 show_max_user_watches, store_max_user_watches);
+
+static inline void get_inotify_dev(struct inotify_device *dev)
+{
+	atomic_inc(&dev->count);
+}
+
+static inline void put_inotify_dev(struct inotify_device *dev)
+{
+	if (atomic_dec_and_test(&dev->count)) {
+		atomic_dec(&dev->user->inotify_devs);
+		free_uid(dev->user);
+		kfree(dev);
+	}
+}
+
+static inline void get_inotify_watch(struct inotify_watch *watch)
+{
+	atomic_inc(&watch->count);
+}
+
+/*
+ * put_inotify_watch - decrements the ref count on a given watch.  cleans up
+ * the watch and its references if the count reaches zero.
+ */
+static inline void put_inotify_watch(struct inotify_watch *watch)
+{
+	if (atomic_dec_and_test(&watch->count)) {
+		put_inotify_dev(watch->dev);
+		iput(watch->inode);
+		kmem_cache_free(watch_cachep, watch);
+	}
+}
+
+/*
+ * kernel_event - create a new kernel event with the given parameters
+ *
+ * This function can sleep.
+ */
+static struct inotify_kernel_event * kernel_event(s32 wd, u32 mask, u32 cookie,
+						  const char *name)
+{
+	struct inotify_kernel_event *kevent;
+
+	kevent = kmem_cache_alloc(event_cachep, GFP_KERNEL);
+	if (unlikely(!kevent))
+		return NULL;
+
+	/* we hand this out to user-space, so zero it just in case */
+	memset(&kevent->event, 0, sizeof(struct inotify_event));
+
+	kevent->event.wd = wd;
+	kevent->event.mask = mask;
+	kevent->event.cookie = cookie;
+
+	INIT_LIST_HEAD(&kevent->list);
+
+	if (name) {
+		size_t len, rem, event_size = sizeof(struct inotify_event);
+
+		/*
+		 * We need to pad the filename so as to properly align an
+		 * array of inotify_event structures.  Because the structure is
+		 * small and the common case is a small filename, we just round
+		 * up to the next multiple of the structure's sizeof.  This is
+		 * simple and safe for all architectures.
+		 */
+		len = strlen(name) + 1;
+		rem = event_size - len;
+		if (len > event_size) {
+			rem = event_size - (len % event_size);
+			if (len % event_size == 0)
+				rem = 0;
+		}
+
+		kevent->name = kmalloc(len + rem, GFP_KERNEL);
+		if (unlikely(!kevent->name)) {
+			kmem_cache_free(event_cachep, kevent);
+			return NULL;
+		}
+		memcpy(kevent->name, name, len);
+		if (rem)
+			memset(kevent->name + len, 0, rem);		
+		kevent->event.len = len + rem;
+	} else {
+		kevent->event.len = 0;
+		kevent->name = NULL;
+	}
+
+	return kevent;
+}
+
+/*
+ * inotify_dev_get_event - return the next event in the given dev's queue
+ *
+ * Caller must hold dev->sem.
+ */
+static inline struct inotify_kernel_event *
+inotify_dev_get_event(struct inotify_device *dev)
+{
+	return list_entry(dev->events.next, struct inotify_kernel_event, list);
+}
+
+/*
+ * inotify_dev_queue_event - add a new event to the given device
+ *
+ * Caller must hold dev->sem.  Can sleep (calls kernel_event()).
+ */
+static void inotify_dev_queue_event(struct inotify_device *dev,
+				    struct inotify_watch *watch, u32 mask,
+				    u32 cookie, const char *name)
+{
+	struct inotify_kernel_event *kevent, *last;
+
+	/* coalescing: drop this event if it is a dupe of the previous */
+	last = inotify_dev_get_event(dev);
+	if (last && last->event.mask == mask && last->event.wd == watch->wd &&
+			last->event.cookie == cookie) {
+		const char *lastname = last->name;
+
+		if (!name && !lastname)
+			return;
+		if (name && lastname && !strcmp(lastname, name))
+			return;
+	}
+
+	/* the queue overflowed and we already sent the Q_OVERFLOW event */
+	if (unlikely(dev->event_count > dev->max_events))
+		return;
+
+	/* if the queue overflows, we need to notify user space */
+	if (unlikely(dev->event_count == dev->max_events))
+		kevent = kernel_event(-1, IN_Q_OVERFLOW, cookie, NULL);
+	else
+		kevent = kernel_event(watch->wd, mask, cookie, name);
+
+	if (unlikely(!kevent))
+		return;
+
+	/* queue the event and wake up anyone waiting */
+	dev->event_count++;
+	dev->queue_size += sizeof(struct inotify_event) + kevent->event.len;
+	list_add_tail(&kevent->list, &dev->events);
+	wake_up_interruptible(&dev->wq);
+}
+
+/*
+ * remove_kevent - cleans up and ultimately frees the given kevent
+ *
+ * Caller must hold dev->sem.
+ */
+static void remove_kevent(struct inotify_device *dev,
+			  struct inotify_kernel_event *kevent)
+{
+	list_del(&kevent->list);
+
+	dev->event_count--;
+	dev->queue_size -= sizeof(struct inotify_event) + kevent->event.len;
+
+	kfree(kevent->name);
+	kmem_cache_free(event_cachep, kevent);
+}
+
+/*
+ * inotify_dev_event_dequeue - destroy an event on the given device
+ *
+ * Caller must hold dev->sem.
+ */
+static void inotify_dev_event_dequeue(struct inotify_device *dev)
+{
+	if (!list_empty(&dev->events)) {
+		struct inotify_kernel_event *kevent;
+		kevent = inotify_dev_get_event(dev);
+		remove_kevent(dev, kevent);
+	}
+}
+
+/*
+ * inotify_dev_get_wd - returns the next WD for use by the given dev
+ *
+ * Callers must hold dev->sem.  This function can sleep.
+ */
+static int inotify_dev_get_wd(struct inotify_device *dev,
+			      struct inotify_watch *watch)
+{
+	int ret;
+
+	do {
+		if (unlikely(!idr_pre_get(&dev->idr, GFP_KERNEL)))
+			return -ENOSPC;
+		ret = idr_get_new(&dev->idr, watch, &watch->wd);
+	} while (ret == -EAGAIN);
+
+	return ret;
+}
+
+/*
+ * create_watch - creates a watch on the given device.
+ *
+ * Callers must hold dev->sem.  Calls inotify_dev_get_wd() so may sleep.
+ * Both 'dev' and 'inode' (by way of nameidata) need to be pinned.
+ */
+static struct inotify_watch *create_watch(struct inotify_device *dev,
+					  u32 mask, struct inode *inode)
+{
+	struct inotify_watch *watch;
+	int ret;
+
+	if (atomic_read(&dev->user->inotify_watches) >= max_user_watches)
+		return ERR_PTR(-ENOSPC);
+
+	watch = kmem_cache_alloc(watch_cachep, GFP_KERNEL);
+	if (unlikely(!watch))
+		return ERR_PTR(-ENOMEM);
+
+	ret = inotify_dev_get_wd(dev, watch);
+	if (unlikely(ret)) {
+		kmem_cache_free(watch_cachep, watch);
+		return ERR_PTR(ret);
+	}
+
+	watch->mask = mask;
+	atomic_set(&watch->count, 0);
+	INIT_LIST_HEAD(&watch->d_list);
+	INIT_LIST_HEAD(&watch->i_list);
+
+	/* save a reference to device and bump the count to make it official */
+	get_inotify_dev(dev);
+	watch->dev = dev;
+
+	/*
+	 * Save a reference to the inode and bump the ref count to make it
+	 * official.  We hold a reference to nameidata, which makes this safe.
+	 */
+	watch->inode = igrab(inode);
+
+	/* bump our own count, corresponding to our entry in dev->watches */
+	get_inotify_watch(watch);
+
+	atomic_inc(&dev->user->inotify_watches);
+
+	return watch;
+}
+
+/*
+ * inotify_find_dev - find the watch associated with the given inode and dev
+ *
+ * Callers must hold inode->inotify_sem.
+ */
+static struct inotify_watch *inode_find_dev(struct inode *inode,
+					    struct inotify_device *dev)
+{
+	struct inotify_watch *watch;
+
+	list_for_each_entry(watch, &inode->inotify_watches, i_list) {
+		if (watch->dev == dev)
+			return watch;
+	}
+
+	return NULL;
+}
+
+/*
+ * remove_watch_no_event - remove_watch() without the IN_IGNORED event.
+ */
+static void remove_watch_no_event(struct inotify_watch *watch,
+				  struct inotify_device *dev)
+{
+	list_del(&watch->i_list);
+	list_del(&watch->d_list);
+
+	atomic_dec(&dev->user->inotify_watches);
+	idr_remove(&dev->idr, watch->wd);
+	put_inotify_watch(watch);
+}
+
+/*
+ * remove_watch - Remove a watch from both the device and the inode.  Sends
+ * the IN_IGNORED event to the given device signifying that the inode is no
+ * longer watched.
+ *
+ * Callers must hold both inode->inotify_sem and dev->sem.  We drop a
+ * reference to the inode before returning.
+ *
+ * The inode is not iput() so as to remain atomic.  If the inode needs to be
+ * iput(), the call returns one.  Otherwise, it returns zero.
+ */
+static void remove_watch(struct inotify_watch *watch,struct inotify_device *dev)
+{
+	inotify_dev_queue_event(dev, watch, IN_IGNORED, 0, NULL);
+	remove_watch_no_event(watch, dev);
+}
+
+/*
+ * inotify_inode_watched - returns nonzero if there are watches on this inode
+ * and zero otherwise.  We call this lockless, we do not care if we race.
+ */
+static inline int inotify_inode_watched(struct inode *inode)
+{
+	return !list_empty(&inode->inotify_watches);
+}
+
+/* Kernel API */
+
+/**
+ * inotify_inode_queue_event - queue an event to all watches on this inode
+ * @inode: inode event is originating from
+ * @mask: event mask describing this event
+ * @cookie: cookie for synchronization, or zero
+ * @name: filename, if any
+ */
+void inotify_inode_queue_event(struct inode *inode, u32 mask, u32 cookie,
+			       const char *name)
+{
+	struct inotify_watch *watch, *next;
+
+	if (!inotify_inode_watched(inode))
+		return;
+
+	down(&inode->inotify_sem);
+	list_for_each_entry_safe(watch, next, &inode->inotify_watches, i_list) {
+		u32 watch_mask = watch->mask;
+		if (watch_mask & mask) {
+			struct inotify_device *dev = watch->dev;
+			get_inotify_watch(watch);
+			down(&dev->sem);
+			inotify_dev_queue_event(dev, watch, mask, cookie, name);
+			if (watch_mask & IN_ONESHOT)
+				remove_watch_no_event(watch, dev);
+			up(&dev->sem);
+			put_inotify_watch(watch);
+		}
+	}
+	up(&inode->inotify_sem);
+}
+EXPORT_SYMBOL_GPL(inotify_inode_queue_event);
+
+/**
+ * inotify_dentry_parent_queue_event - queue an event to a dentry's parent
+ * @dentry: the dentry in question, we queue against this dentry's parent
+ * @mask: event mask describing this event
+ * @cookie: cookie for synchronization, or zero
+ * @name: filename, if any
+ */
+void inotify_dentry_parent_queue_event(struct dentry *dentry, u32 mask,
+				       u32 cookie, const char *name)
+{
+	struct dentry *parent;
+	struct inode *inode;
+
+	spin_lock(&dentry->d_lock);
+	parent = dentry->d_parent;
+	inode = parent->d_inode;
+
+	if (inotify_inode_watched(inode)) {
+		dget(parent);
+		spin_unlock(&dentry->d_lock);
+		inotify_inode_queue_event(inode, mask, cookie, name);
+		dput(parent);
+	} else
+		spin_unlock(&dentry->d_lock);
+}
+EXPORT_SYMBOL_GPL(inotify_dentry_parent_queue_event);
+
+/**
+ * inotify_get_cookie - return a unique cookie for use in synchronizing events.
+ */
+u32 inotify_get_cookie(void)
+{
+	return atomic_inc_return(&inotify_cookie);
+}
+EXPORT_SYMBOL_GPL(inotify_get_cookie);
+
+/**
+ * inotify_unmount_inodes - an sb is unmounting.  handle any watched inodes.
+ * @list: list of inodes being unmounted (sb->s_inodes)
+ *
+ * Called with inode_lock held, protecting the unmounting super block's list
+ * of inodes, and with iprune_sem held, keeping shrink_icache_memory() at bay.
+ * We temporarily drop inode_lock, however, and CAN block.
+ */
+void inotify_unmount_inodes(struct list_head *list)
+{
+	struct inode *inode, *next_i;
+
+	list_for_each_entry_safe(inode, next_i, list, i_sb_list) {
+		struct inotify_watch *watch, *next_w;
+		struct list_head *watches;
+
+		/*
+		 * We cannot __iget() an inode in state I_CLEAR or I_FREEING,
+		 * which is fine because by that point the inode cannot have
+		 * any associated watches.
+		 */
+		if (inode->i_state & (I_CLEAR | I_FREEING))
+			continue;
+
+		/* In case the remove_watch() drops a reference */
+		__iget(inode);
+
+		/*
+		 * We can safely drop inode_lock here because the per-sb list
+		 * of inodes must not change during unmount and iprune_sem
+		 * keeps shrink_icache_memory() away.
+		 */
+		spin_unlock(&inode_lock);
+
+		/* for each watch, send IN_UNMOUNT and then remove it */
+		down(&inode->inotify_sem);
+		watches = &inode->inotify_watches;
+		list_for_each_entry_safe(watch, next_w, watches, i_list) {
+			struct inotify_device *dev = watch->dev;
+			down(&dev->sem);
+			inotify_dev_queue_event(dev, watch, IN_UNMOUNT,0,NULL);
+			remove_watch(watch, dev);
+			up(&dev->sem);
+		}
+		up(&inode->inotify_sem);
+		iput(inode);		
+
+		spin_lock(&inode_lock);
+	}
+}
+EXPORT_SYMBOL_GPL(inotify_unmount_inodes);
+
+/**
+ * inotify_inode_is_dead - an inode has been deleted, cleanup any watches
+ * @inode: inode that is about to be removed
+ */
+void inotify_inode_is_dead(struct inode *inode)
+{
+	struct inotify_watch *watch, *next;
+
+	down(&inode->inotify_sem);
+	list_for_each_entry_safe(watch, next, &inode->inotify_watches, i_list) {
+		struct inotify_device *dev = watch->dev;
+		down(&dev->sem);
+		remove_watch(watch, dev);
+		up(&dev->sem);
+	}
+	up(&inode->inotify_sem);
+}
+EXPORT_SYMBOL_GPL(inotify_inode_is_dead);
+
+/* Device Interface */
+
+static unsigned int inotify_poll(struct file *file, poll_table *wait)
+{
+	struct inotify_device *dev = file->private_data;
+	int ret = 0;
+
+	poll_wait(file, &dev->wq, wait);
+	down(&dev->sem);
+	if (!list_empty(&dev->events))
+		ret = POLLIN | POLLRDNORM;
+	up(&dev->sem);
+
+	return ret;
+}
+
+static ssize_t inotify_read(struct file *file, char __user *buf,
+			    size_t count, loff_t *pos)
+{
+	size_t event_size = sizeof (struct inotify_event);
+	struct inotify_device *dev;
+	char __user *start;
+	int ret;
+	DEFINE_WAIT(wait);
+
+	start = buf;
+	dev = file->private_data;
+
+	while (1) {
+		int events;
+
+		prepare_to_wait(&dev->wq, &wait, TASK_INTERRUPTIBLE);
+
+		down(&dev->sem);
+		events = !list_empty(&dev->events);
+		up(&dev->sem);
+		if (events) {
+			ret = 0;
+			break;
+		}
+
+		if (file->f_flags & O_NONBLOCK) {
+			ret = -EAGAIN;
+			break;
+		}
+
+		if (signal_pending(current)) {
+			ret = -EINTR;
+			break;
+		}
+
+		schedule();
+	}
+
+	finish_wait(&dev->wq, &wait);
+	if (ret)
+		return ret;
+
+	down(&dev->sem);
+	while (1) {
+		struct inotify_kernel_event *kevent;
+
+		ret = buf - start;
+		if (list_empty(&dev->events))
+			break;
+
+		kevent = inotify_dev_get_event(dev);
+		if (event_size + kevent->event.len > count)
+			break;
+
+		if (copy_to_user(buf, &kevent->event, event_size)) {
+			ret = -EFAULT;
+			break;
+		}
+		buf += event_size;
+		count -= event_size;
+
+		if (kevent->name) {
+			if (copy_to_user(buf, kevent->name, kevent->event.len)){
+				ret = -EFAULT;
+				break;
+			}
+			buf += kevent->event.len;
+			count -= kevent->event.len;
+		}
+
+		remove_kevent(dev, kevent);
+	}
+	up(&dev->sem);
+
+	return ret;
+}
+
+static int inotify_open(struct inode *inode, struct file *file)
+{
+	struct inotify_device *dev;
+	struct user_struct *user;
+	int ret;
+
+	user = get_uid(current->user);
+
+	if (unlikely(atomic_read(&user->inotify_devs) >= max_user_devices)) {
+		ret = -EMFILE;
+		goto out_err;
+	}
+
+	dev = kmalloc(sizeof(struct inotify_device), GFP_KERNEL);
+	if (unlikely(!dev)) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	idr_init(&dev->idr);
+	INIT_LIST_HEAD(&dev->events);
+	INIT_LIST_HEAD(&dev->watches);
+	init_waitqueue_head(&dev->wq);
+	sema_init(&dev->sem, 1);
+	dev->event_count = 0;
+	dev->queue_size = 0;
+	dev->max_events = max_queued_events;
+	dev->user = user;
+	atomic_set(&dev->count, 0);
+
+	get_inotify_dev(dev);
+	atomic_inc(&user->inotify_devs);
+
+	file->private_data = dev;
+
+	ret = nonseekable_open(inode, file);
+	if (ret)
+		goto out_err;
+
+	return 0;
+out_err:
+	free_uid(user);
+	return ret;
+}
+
+static int inotify_release(struct inode *ignored, struct file *file)
+{
+	struct inotify_device *dev = file->private_data;
+
+	/*
+	 * Destroy all of the watches on this device.  Unfortunately, not very
+	 * pretty.  We cannot do a simple iteration over the list, because we
+	 * do not know the inode until we iterate to the watch.  But we need to
+	 * hold inode->inotify_sem before dev->sem.  The following works.
+	 */
+	while (1) {
+		struct inotify_watch *watch;
+		struct list_head *watches;
+		struct inode *inode;
+
+		down(&dev->sem);
+		watches = &dev->watches;
+		if (list_empty(watches)) {
+			up(&dev->sem);
+			break;
+		}
+		watch = list_entry(watches->next, struct inotify_watch, d_list);
+		get_inotify_watch(watch);
+		up(&dev->sem);
+
+		inode = watch->inode;
+		down(&inode->inotify_sem);
+		down(&dev->sem);
+		remove_watch_no_event(watch, dev);
+		up(&dev->sem);
+		up(&inode->inotify_sem);
+		put_inotify_watch(watch);
+	}
+
+	/* destroy all of the events on this device */
+	down(&dev->sem);
+	while (!list_empty(&dev->events))
+		inotify_dev_event_dequeue(dev);
+	up(&dev->sem);
+
+	/* free this device: the put matching the get in inotify_open() */
+	put_inotify_dev(dev);
+
+	return 0;
+}
+
+static int inotify_add_watch(struct inotify_device *dev, int fd, u32 mask)
+{
+	struct inotify_watch *watch, *old;
+	struct inode *inode;
+	struct file *filp;
+	int ret;
+
+	filp = fget(fd);
+	if (!filp)
+		return -EBADF;
+	inode = filp->f_dentry->d_inode;
+
+	down(&inode->inotify_sem);
+	down(&dev->sem);
+
+	/* don't let user-space set invalid bits: we don't want flags set */
+	mask &= IN_ALL_EVENTS;
+	if (!mask) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/*
+	 * Handle the case of re-adding a watch on an (inode,dev) pair that we
+	 * are already watching.  We just update the mask and return its wd.
+	 */
+	old = inode_find_dev(inode, dev);
+	if (unlikely(old)) {
+		old->mask = mask;
+		ret = old->wd;
+		goto out;
+	}
+
+	watch = create_watch(dev, mask, inode);
+	if (unlikely(IS_ERR(watch))) {
+		ret = PTR_ERR(watch);
+		goto out;
+	}
+
+	/* Add the watch to the device's and the inode's list */
+	list_add(&watch->d_list, &dev->watches);
+	list_add(&watch->i_list, &inode->inotify_watches);
+	ret = watch->wd;
+
+out:
+	up(&dev->sem);
+	up(&inode->inotify_sem);
+	fput(filp);
+
+	return ret;
+}
+
+/*
+ * inotify_ignore - handle the INOTIFY_IGNORE ioctl, asking that a given wd be
+ * removed from the device.
+ *
+ * Can sleep.
+ */
+static int inotify_ignore(struct inotify_device *dev, s32 wd)
+{
+	struct inotify_watch *watch;
+	struct inode *inode;
+
+	down(&dev->sem);
+	watch = idr_find(&dev->idr, wd);
+	if (unlikely(!watch)) {
+		up(&dev->sem);
+		return -EINVAL;
+	}
+	get_inotify_watch(watch);
+	inode = watch->inode;
+	up(&dev->sem);
+
+	down(&inode->inotify_sem);
+	down(&dev->sem);
+
+	/* make sure that we did not race */
+	watch = idr_find(&dev->idr, wd);
+	if (likely(watch))
+		remove_watch(watch, dev);
+
+	up(&dev->sem);
+	up(&inode->inotify_sem);
+	put_inotify_watch(watch);
+
+	return 0;
+}
+
+static long inotify_ioctl(struct file *file, unsigned int cmd,
+			  unsigned long arg)
+{
+	struct inotify_device *dev;
+	struct inotify_watch_request request;
+	void __user *p;
+	int ret = -ENOTTY;
+	s32 wd;
+
+	dev = file->private_data;
+	p = (void __user *) arg;
+
+	switch (cmd) {
+	case INOTIFY_WATCH:
+		if (unlikely(copy_from_user(&request, p, sizeof (request)))) {
+			ret = -EFAULT;
+			break;
+		}
+		ret = inotify_add_watch(dev, request.fd, request.mask);
+		break;
+	case INOTIFY_IGNORE:
+		if (unlikely(get_user(wd, (int __user *) p))) {
+			ret = -EFAULT;
+			break;
+		}
+		ret = inotify_ignore(dev, wd);
+		break;
+	case FIONREAD:
+		ret = put_user(dev->queue_size, (int __user *) p);
+		break;
+	}
+
+	return ret;
+}
+
+static struct file_operations inotify_fops = {
+	.owner		= THIS_MODULE,
+	.poll		= inotify_poll,
+	.read		= inotify_read,
+	.open		= inotify_open,
+	.release	= inotify_release,
+	.unlocked_ioctl	= inotify_ioctl,
+	.compat_ioctl	= inotify_ioctl,
+};
+
+static struct miscdevice inotify_device = {
+	.minor  = MISC_DYNAMIC_MINOR,
+	.name	= "inotify",
+	.fops	= &inotify_fops,
+};
+
+/*
+ * inotify_init - Our initialization function.  Note that we cannnot return
+ * error because we have compiled-in VFS hooks.  So an (unlikely) failure here
+ * must result in panic().
+ */
+static int __init inotify_init(void)
+{
+	struct class_device *class;
+	int ret;
+
+	ret = misc_register(&inotify_device);
+	if (unlikely(ret))
+		panic("inotify: misc_register returned %d\n", ret);
+
+	max_queued_events = 8192;
+	max_user_devices = 128;
+	max_user_watches = 8192;
+
+	class = inotify_device.class;
+	class_device_create_file(class, &class_device_attr_max_queued_events);
+	class_device_create_file(class, &class_device_attr_max_user_devices);
+	class_device_create_file(class, &class_device_attr_max_user_watches);
+
+	atomic_set(&inotify_cookie, 0);
+
+	watch_cachep = kmem_cache_create("inotify_watch_cache",
+					 sizeof(struct inotify_watch),
+					 0, SLAB_PANIC, NULL, NULL);
+	event_cachep = kmem_cache_create("inotify_event_cache",
+					 sizeof(struct inotify_kernel_event),
+					 0, SLAB_PANIC, NULL, NULL);
+
+	printk(KERN_INFO "inotify device minor=%d\n", inotify_device.minor);
+
+	return 0;
+}
+
+module_init(inotify_init);
diff -urN linux-2.6.13-rc1/fs/Kconfig linux/fs/Kconfig
--- linux-2.6.13-rc1/fs/Kconfig	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/Kconfig	2005-07-01 16:13:14.000000000 -0400
@@ -356,6 +356,19 @@
 	  If you don't know whether you need it, then you don't need it:
 	  answer N.
 
+config INOTIFY
+	bool "Inotify file change notification support"
+	default y
+	---help---
+	  Say Y here to enable inotify support and the /dev/inotify character
+	  device.  Inotify is a file change notification system and a
+	  replacement for dnotify.  Inotify fixes numerous shortcomings in
+	  dnotify and introduces several new features.  It allows monitoring
+	  of both files and directories via a single open fd.  Multiple file
+	  events are supported.
+
+	  If unsure, say Y.
+
 config QUOTA
 	bool "Quota support"
 	help
diff -urN linux-2.6.13-rc1/fs/Makefile linux/fs/Makefile
--- linux-2.6.13-rc1/fs/Makefile	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/Makefile	2005-07-01 16:13:14.000000000 -0400
@@ -12,6 +12,7 @@
 		seq_file.o xattr.o libfs.o fs-writeback.o mpage.o direct-io.o \
 		ioprio.o
 
+obj-$(CONFIG_INOTIFY)		+= inotify.o
 obj-$(CONFIG_EPOLL)		+= eventpoll.o
 obj-$(CONFIG_COMPAT)		+= compat.o
 
diff -urN linux-2.6.13-rc1/fs/namei.c linux/fs/namei.c
--- linux-2.6.13-rc1/fs/namei.c	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/namei.c	2005-07-01 16:13:14.000000000 -0400
@@ -21,7 +21,7 @@
 #include <linux/namei.h>
 #include <linux/quotaops.h>
 #include <linux/pagemap.h>
-#include <linux/dnotify.h>
+#include <linux/fsnotify.h>
 #include <linux/smp_lock.h>
 #include <linux/personality.h>
 #include <linux/security.h>
@@ -1312,7 +1312,7 @@
 	DQUOT_INIT(dir);
 	error = dir->i_op->create(dir, dentry, mode, nd);
 	if (!error) {
-		inode_dir_notify(dir, DN_CREATE);
+		fsnotify_create(dir, dentry->d_name.name);
 		security_inode_post_create(dir, dentry, mode);
 	}
 	return error;
@@ -1637,7 +1637,7 @@
 	DQUOT_INIT(dir);
 	error = dir->i_op->mknod(dir, dentry, mode, dev);
 	if (!error) {
-		inode_dir_notify(dir, DN_CREATE);
+		fsnotify_create(dir, dentry->d_name.name);
 		security_inode_post_mknod(dir, dentry, mode, dev);
 	}
 	return error;
@@ -1710,7 +1710,7 @@
 	DQUOT_INIT(dir);
 	error = dir->i_op->mkdir(dir, dentry, mode);
 	if (!error) {
-		inode_dir_notify(dir, DN_CREATE);
+		fsnotify_mkdir(dir, dentry->d_name.name);
 		security_inode_post_mkdir(dir,dentry, mode);
 	}
 	return error;
@@ -1801,7 +1801,7 @@
 	}
 	up(&dentry->d_inode->i_sem);
 	if (!error) {
-		inode_dir_notify(dir, DN_DELETE);
+		fsnotify_rmdir(dentry, dentry->d_inode, dir);
 		d_delete(dentry);
 	}
 	dput(dentry);
@@ -1874,9 +1874,10 @@
 
 	/* We don't d_delete() NFS sillyrenamed files--they still exist. */
 	if (!error && !(dentry->d_flags & DCACHE_NFSFS_RENAMED)) {
+		fsnotify_unlink(dentry, dir);
 		d_delete(dentry);
-		inode_dir_notify(dir, DN_DELETE);
 	}
+
 	return error;
 }
 
@@ -1950,7 +1951,7 @@
 	DQUOT_INIT(dir);
 	error = dir->i_op->symlink(dir, dentry, oldname);
 	if (!error) {
-		inode_dir_notify(dir, DN_CREATE);
+		fsnotify_create(dir, dentry->d_name.name);
 		security_inode_post_symlink(dir, dentry, oldname);
 	}
 	return error;
@@ -2023,7 +2024,7 @@
 	error = dir->i_op->link(old_dentry, dir, new_dentry);
 	up(&old_dentry->d_inode->i_sem);
 	if (!error) {
-		inode_dir_notify(dir, DN_CREATE);
+		fsnotify_create(dir, new_dentry->d_name.name);
 		security_inode_post_link(old_dentry, dir, new_dentry);
 	}
 	return error;
@@ -2187,6 +2188,7 @@
 {
 	int error;
 	int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);
+	const char *old_name;
 
 	if (old_dentry->d_inode == new_dentry->d_inode)
  		return 0;
@@ -2208,18 +2210,18 @@
 	DQUOT_INIT(old_dir);
 	DQUOT_INIT(new_dir);
 
+	old_name = fsnotify_oldname_init(old_dentry->d_name.name);
+
 	if (is_dir)
 		error = vfs_rename_dir(old_dir,old_dentry,new_dir,new_dentry);
 	else
 		error = vfs_rename_other(old_dir,old_dentry,new_dir,new_dentry);
 	if (!error) {
-		if (old_dir == new_dir)
-			inode_dir_notify(old_dir, DN_RENAME);
-		else {
-			inode_dir_notify(old_dir, DN_DELETE);
-			inode_dir_notify(new_dir, DN_CREATE);
-		}
+		const char *new_name = old_dentry->d_name.name;
+		fsnotify_move(old_dir, new_dir, old_name, new_name, is_dir);
 	}
+	fsnotify_oldname_free(old_name);
+
 	return error;
 }
 
diff -urN linux-2.6.13-rc1/fs/nfsd/vfs.c linux/fs/nfsd/vfs.c
--- linux-2.6.13-rc1/fs/nfsd/vfs.c	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/nfsd/vfs.c	2005-07-01 16:13:29.000000000 -0400
@@ -45,7 +45,7 @@
 #endif /* CONFIG_NFSD_V3 */
 #include <linux/nfsd/nfsfh.h>
 #include <linux/quotaops.h>
-#include <linux/dnotify.h>
+#include <linux/fsnotify.h>
 #include <linux/posix_acl.h>
 #include <linux/posix_acl_xattr.h>
 #ifdef CONFIG_NFSD_V4
@@ -860,7 +860,7 @@
 		nfsdstats.io_read += err;
 		*count = err;
 		err = 0;
-		dnotify_parent(file->f_dentry, DN_ACCESS);
+		fsnotify_access(file->f_dentry);
 	} else 
 		err = nfserrno(err);
 out:
@@ -916,7 +916,7 @@
 	set_fs(oldfs);
 	if (err >= 0) {
 		nfsdstats.io_write += cnt;
-		dnotify_parent(file->f_dentry, DN_MODIFY);
+		fsnotify_modify(file->f_dentry);
 	}
 
 	/* clear setuid/setgid flag after write */
diff -urN linux-2.6.13-rc1/fs/open.c linux/fs/open.c
--- linux-2.6.13-rc1/fs/open.c	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/open.c	2005-07-01 16:14:35.000000000 -0400
@@ -10,7 +10,7 @@
 #include <linux/file.h>
 #include <linux/smp_lock.h>
 #include <linux/quotaops.h>
-#include <linux/dnotify.h>
+#include <linux/fsnotify.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/tty.h>
@@ -951,6 +951,7 @@
 				put_unused_fd(fd);
 				fd = PTR_ERR(f);
 			} else {
+				fsnotify_open(f->f_dentry);
 				fd_install(fd, f);
 			}
 		}
diff -urN linux-2.6.13-rc1/fs/read_write.c linux/fs/read_write.c
--- linux-2.6.13-rc1/fs/read_write.c	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/read_write.c	2005-07-01 16:13:14.000000000 -0400
@@ -10,7 +10,7 @@
 #include <linux/file.h>
 #include <linux/uio.h>
 #include <linux/smp_lock.h>
-#include <linux/dnotify.h>
+#include <linux/fsnotify.h>
 #include <linux/security.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
@@ -252,7 +252,7 @@
 			else
 				ret = do_sync_read(file, buf, count, pos);
 			if (ret > 0) {
-				dnotify_parent(file->f_dentry, DN_ACCESS);
+				fsnotify_access(file->f_dentry);
 				current->rchar += ret;
 			}
 			current->syscr++;
@@ -303,7 +303,7 @@
 			else
 				ret = do_sync_write(file, buf, count, pos);
 			if (ret > 0) {
-				dnotify_parent(file->f_dentry, DN_MODIFY);
+				fsnotify_modify(file->f_dentry);
 				current->wchar += ret;
 			}
 			current->syscw++;
@@ -539,9 +539,12 @@
 out:
 	if (iov != iovstack)
 		kfree(iov);
-	if ((ret + (type == READ)) > 0)
-		dnotify_parent(file->f_dentry,
-				(type == READ) ? DN_ACCESS : DN_MODIFY);
+	if ((ret + (type == READ)) > 0) {
+		if (type == READ)
+			fsnotify_access(file->f_dentry);
+		else
+			fsnotify_modify(file->f_dentry);
+	}
 	return ret;
 Efault:
 	ret = -EFAULT;
diff -urN linux-2.6.13-rc1/fs/sysfs/file.c linux/fs/sysfs/file.c
--- linux-2.6.13-rc1/fs/sysfs/file.c	2005-07-01 16:12:13.000000000 -0400
+++ linux/fs/sysfs/file.c	2005-07-01 16:13:14.000000000 -0400
@@ -3,7 +3,7 @@
  */
 
 #include <linux/module.h>
-#include <linux/dnotify.h>
+#include <linux/fsnotify.h>
 #include <linux/kobject.h>
 #include <linux/namei.h>
 #include <asm/uaccess.h>
@@ -391,9 +391,6 @@
  * sysfs_update_file - update the modified timestamp on an object attribute.
  * @kobj: object we're acting for.
  * @attr: attribute descriptor.
- *
- * Also call dnotify for the dentry, which lots of userspace programs
- * use.
  */
 int sysfs_update_file(struct kobject * kobj, const struct attribute * attr)
 {
@@ -408,7 +405,7 @@
 		if (victim->d_inode && 
 		    (victim->d_parent->d_inode == dir->d_inode)) {
 			victim->d_inode->i_mtime = CURRENT_TIME;
-			dnotify_parent(victim, DN_MODIFY);
+			fsnotify_modify(victim);
 
 			/**
 			 * Drop reference from initial sysfs_get_dentry().
diff -urN linux-2.6.13-rc1/fs/xattr.c linux/fs/xattr.c
--- linux-2.6.13-rc1/fs/xattr.c	2005-07-01 16:10:51.000000000 -0400
+++ linux/fs/xattr.c	2005-07-01 16:13:14.000000000 -0400
@@ -16,6 +16,7 @@
 #include <linux/security.h>
 #include <linux/syscalls.h>
 #include <linux/module.h>
+#include <linux/fsnotify.h>
 #include <asm/uaccess.h>
 
 /*
@@ -57,8 +58,10 @@
 		if (error)
 			goto out;
 		error = d->d_inode->i_op->setxattr(d, kname, kvalue, size, flags);
-		if (!error)
+		if (!error) {
+			fsnotify_xattr(d);
 			security_inode_post_setxattr(d, kname, kvalue, size, flags);
+		}
 out:
 		up(&d->d_inode->i_sem);
 	}
diff -urN linux-2.6.13-rc1/include/linux/fs.h linux/include/linux/fs.h
--- linux-2.6.13-rc1/include/linux/fs.h	2005-07-01 16:12:13.000000000 -0400
+++ linux/include/linux/fs.h	2005-07-01 16:13:14.000000000 -0400
@@ -474,6 +474,11 @@
 	struct dnotify_struct	*i_dnotify; /* for directory notifications */
 #endif
 
+#ifdef CONFIG_INOTIFY
+	struct list_head	inotify_watches; /* watches on this inode */
+	struct semaphore	inotify_sem;	/* protects the watches list */
+#endif
+
 	unsigned long		i_state;
 	unsigned long		dirtied_when;	/* jiffies of first dirtying */
 
@@ -1393,7 +1398,6 @@
 extern int do_remount_sb(struct super_block *sb, int flags,
 			 void *data, int force);
 extern sector_t bmap(struct inode *, sector_t);
-extern int setattr_mask(unsigned int);
 extern int notify_change(struct dentry *, struct iattr *);
 extern int permission(struct inode *, int, struct nameidata *);
 extern int generic_permission(struct inode *, int,
diff -urN linux-2.6.13-rc1/include/linux/fsnotify.h linux/include/linux/fsnotify.h
--- linux-2.6.13-rc1/include/linux/fsnotify.h	1969-12-31 19:00:00.000000000 -0500
+++ linux/include/linux/fsnotify.h	2005-07-01 16:13:14.000000000 -0400
@@ -0,0 +1,257 @@
+#ifndef _LINUX_FS_NOTIFY_H
+#define _LINUX_FS_NOTIFY_H
+
+/*
+ * include/linux/fsnotify.h - generic hooks for filesystem notification, to
+ * reduce in-source duplication from both dnotify and inotify.
+ *
+ * We don't compile any of this away in some complicated menagerie of ifdefs.
+ * Instead, we rely on the code inside to optimize away as needed.
+ *
+ * (C) Copyright 2005 Robert Love
+ */
+
+#ifdef __KERNEL__
+
+#include <linux/dnotify.h>
+#include <linux/inotify.h>
+
+/*
+ * fsnotify_move - file old_name at old_dir was moved to new_name at new_dir
+ */
+static inline void fsnotify_move(struct inode *old_dir, struct inode *new_dir,
+				 const char *old_name, const char *new_name,
+				 int isdir)
+{
+	u32 cookie = inotify_get_cookie();
+
+	if (old_dir == new_dir)
+		inode_dir_notify(old_dir, DN_RENAME);
+	else {
+		inode_dir_notify(old_dir, DN_DELETE);
+		inode_dir_notify(new_dir, DN_CREATE);
+	}
+
+	if (isdir)
+		isdir = IN_ISDIR;
+	inotify_inode_queue_event(old_dir, IN_MOVED_FROM|isdir,cookie,old_name);
+	inotify_inode_queue_event(new_dir, IN_MOVED_TO|isdir, cookie, new_name);
+}
+
+/*
+ * fsnotify_unlink - file was unlinked
+ */
+static inline void fsnotify_unlink(struct dentry *dentry, struct inode *dir)
+{
+	struct inode *inode = dentry->d_inode;
+
+	inode_dir_notify(dir, DN_DELETE);
+	inotify_inode_queue_event(dir, IN_DELETE, 0, dentry->d_name.name);
+	inotify_inode_queue_event(inode, IN_DELETE_SELF, 0, NULL);
+
+	inotify_inode_is_dead(inode);
+}
+
+/*
+ * fsnotify_rmdir - directory was removed
+ */
+static inline void fsnotify_rmdir(struct dentry *dentry, struct inode *inode,
+				  struct inode *dir)
+{
+	inode_dir_notify(dir, DN_DELETE);
+	inotify_inode_queue_event(dir,IN_DELETE|IN_ISDIR,0,dentry->d_name.name);
+	inotify_inode_queue_event(inode, IN_DELETE_SELF | IN_ISDIR, 0, NULL);
+	inotify_inode_is_dead(inode);
+}
+
+/*
+ * fsnotify_create - 'name' was linked in
+ */
+static inline void fsnotify_create(struct inode *inode, const char *name)
+{
+	inode_dir_notify(inode, DN_CREATE);
+	inotify_inode_queue_event(inode, IN_CREATE, 0, name);
+}
+
+/*
+ * fsnotify_mkdir - directory 'name' was created
+ */
+static inline void fsnotify_mkdir(struct inode *inode, const char *name)
+{
+	inode_dir_notify(inode, DN_CREATE);
+	inotify_inode_queue_event(inode, IN_CREATE | IN_ISDIR, 0, name);
+}
+
+/*
+ * fsnotify_access - file was read
+ */
+static inline void fsnotify_access(struct dentry *dentry)
+{
+	struct inode *inode = dentry->d_inode;
+	u32 mask = IN_ACCESS;
+
+	if (S_ISDIR(inode->i_mode))
+		mask |= IN_ISDIR;
+
+	dnotify_parent(dentry, DN_ACCESS);
+	inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name);
+	inotify_inode_queue_event(inode, mask, 0, NULL);
+}
+
+/*
+ * fsnotify_modify - file was modified
+ */
+static inline void fsnotify_modify(struct dentry *dentry)
+{
+	struct inode *inode = dentry->d_inode;
+	u32 mask = IN_MODIFY;
+
+	if (S_ISDIR(inode->i_mode))
+		mask |= IN_ISDIR;
+
+	dnotify_parent(dentry, DN_MODIFY);
+	inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name);
+	inotify_inode_queue_event(inode, mask, 0, NULL);
+}
+
+/*
+ * fsnotify_open - file was opened
+ */
+static inline void fsnotify_open(struct dentry *dentry)
+{
+	struct inode *inode = dentry->d_inode;
+	u32 mask = IN_OPEN;
+
+	if (S_ISDIR(inode->i_mode))
+		mask |= IN_ISDIR;
+
+	inotify_inode_queue_event(inode, mask, 0, NULL);
+	inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name);
+}
+
+/*
+ * fsnotify_close - file was closed
+ */
+static inline void fsnotify_close(struct file *file)
+{
+	struct dentry *dentry = file->f_dentry;
+	struct inode *inode = dentry->d_inode;
+	const char *name = dentry->d_name.name;
+	mode_t mode = file->f_mode;
+	u32 mask = (mode & FMODE_WRITE) ? IN_CLOSE_WRITE : IN_CLOSE_NOWRITE;
+
+	if (S_ISDIR(inode->i_mode))
+		mask |= IN_ISDIR;
+
+	inotify_dentry_parent_queue_event(dentry, mask, 0, name);
+	inotify_inode_queue_event(inode, mask, 0, NULL);
+}
+
+/*
+ * fsnotify_xattr - extended attributes were changed
+ */
+static inline void fsnotify_xattr(struct dentry *dentry)
+{
+	struct inode *inode = dentry->d_inode;
+	u32 mask = IN_ATTRIB;
+
+	if (S_ISDIR(inode->i_mode))
+		mask |= IN_ISDIR;
+
+	inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name);
+	inotify_inode_queue_event(inode, mask, 0, NULL);
+}
+
+/*
+ * fsnotify_change - notify_change event.  file was modified and/or metadata
+ * was changed.
+ */
+static inline void fsnotify_change(struct dentry *dentry, unsigned int ia_valid)
+{
+	struct inode *inode = dentry->d_inode;
+	int dn_mask = 0;
+	u32 in_mask = 0;
+
+	if (ia_valid & ATTR_UID) {
+		in_mask |= IN_ATTRIB;
+		dn_mask |= DN_ATTRIB;
+	}
+	if (ia_valid & ATTR_GID) {
+		in_mask |= IN_ATTRIB;
+		dn_mask |= DN_ATTRIB;
+	}
+	if (ia_valid & ATTR_SIZE) {
+		in_mask |= IN_MODIFY;
+		dn_mask |= DN_MODIFY;
+	}
+	/* both times implies a utime(s) call */
+	if ((ia_valid & (ATTR_ATIME | ATTR_MTIME)) == (ATTR_ATIME | ATTR_MTIME))
+	{
+		in_mask |= IN_ATTRIB;
+		dn_mask |= DN_ATTRIB;
+	} else if (ia_valid & ATTR_ATIME) {
+		in_mask |= IN_ACCESS;
+		dn_mask |= DN_ACCESS;
+	} else if (ia_valid & ATTR_MTIME) {
+		in_mask |= IN_MODIFY;
+		dn_mask |= DN_MODIFY;
+	}
+	if (ia_valid & ATTR_MODE) {
+		in_mask |= IN_ATTRIB;
+		dn_mask |= DN_ATTRIB;
+	}
+
+	if (dn_mask)
+		dnotify_parent(dentry, dn_mask);
+	if (in_mask) {
+		if (S_ISDIR(inode->i_mode))
+			in_mask |= IN_ISDIR;
+		inotify_inode_queue_event(inode, in_mask, 0, NULL);
+		inotify_dentry_parent_queue_event(dentry, in_mask, 0,
+						  dentry->d_name.name);
+	}
+}
+
+#ifdef CONFIG_INOTIFY	/* inotify helpers */
+
+/*
+ * fsnotify_oldname_init - save off the old filename before we change it
+ *
+ * XXX: This could be kstrdup if only we could add that to lib/string.c
+ */
+static inline const char *fsnotify_oldname_init(const char *name)
+{
+	size_t len;
+	char *buf;
+
+	len = strlen(name) + 1;
+	buf = kmalloc(len, GFP_KERNEL);
+	if (likely(buf))
+		memcpy(buf, name, len);
+	return buf;
+}
+
+/*
+ * fsnotify_oldname_free - free the name we got from fsnotify_oldname_init
+ */
+static inline void fsnotify_oldname_free(const char *old_name)
+{
+	kfree(old_name);
+}
+
+#else	/* CONFIG_INOTIFY */
+
+static inline const char *fsnotify_oldname_init(const char *name)
+{
+	return NULL;
+}
+
+static inline void fsnotify_oldname_free(const char *old_name)
+{
+}
+
+#endif	/* ! CONFIG_INOTIFY */
+
+#endif	/* __KERNEL__ */
+
+#endif	/* _LINUX_FS_NOTIFY_H */
diff -urN linux-2.6.13-rc1/include/linux/inotify.h linux/include/linux/inotify.h
--- linux-2.6.13-rc1/include/linux/inotify.h	1969-12-31 19:00:00.000000000 -0500
+++ linux/include/linux/inotify.h	2005-07-01 16:13:14.000000000 -0400
@@ -0,0 +1,124 @@
+/*
+ * Inode based directory notification for Linux
+ *
+ * Copyright (C) 2005 John McCutchan
+ */
+
+#ifndef _LINUX_INOTIFY_H
+#define _LINUX_INOTIFY_H
+
+#include <linux/types.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, ..., relative to the wd.
+ */
+struct inotify_event {
+	__s32		wd;		/* watch descriptor */
+	__u32		mask;		/* watch mask */
+	__u32		cookie;		/* cookie to synchronize two events */
+	__u32		len;		/* length (including nulls) of name */
+	char		name[0];	/* stub for possible name */
+};
+
+/*
+ * struct inotify_watch_request - represents a watch request
+ *
+ * Pass to the inotify device via the INOTIFY_WATCH ioctl
+ */
+struct inotify_watch_request {
+	int		fd;		/* fd of filename to watch */
+	__u32		mask;		/* event mask */
+};
+
+/* the following are legal, implemented events that user-space can watch for */
+#define IN_ACCESS		0x00000001	/* File was accessed */
+#define IN_MODIFY		0x00000002	/* File was modified */
+#define IN_ATTRIB		0x00000004	/* Metadata changed */
+#define IN_CLOSE_WRITE		0x00000008	/* Writtable file was closed */
+#define IN_CLOSE_NOWRITE	0x00000010	/* Unwrittable file closed */
+#define IN_OPEN			0x00000020	/* File was opened */
+#define IN_MOVED_FROM		0x00000040	/* File was moved from X */
+#define IN_MOVED_TO		0x00000080	/* File was moved to Y */
+#define IN_CREATE		0x00000100	/* Subfile was created */
+#define IN_DELETE		0x00000200	/* Subfile was deleted */
+#define IN_DELETE_SELF		0x00000400	/* Self was deleted */
+
+/* the following are legal events.  they are sent as needed to any watch */
+#define IN_UNMOUNT		0x00002000	/* Backing fs was unmounted */
+#define IN_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
+#define IN_IGNORED		0x00008000	/* File was ignored */
+
+/* helper events */
+#define IN_CLOSE		(IN_CLOSE_WRITE | IN_CLOSE_NOWRITE) /* close */
+#define IN_MOVE			(IN_MOVED_FROM | IN_MOVED_TO) /* moves */
+
+/* special flags */
+#define IN_ISDIR		0x40000000	/* event occurred against dir */
+#define IN_ONESHOT		0x80000000	/* only send event once */
+
+/*
+ * All of the events - we build the list by hand so that we can add flags in
+ * the future and not break backward compatibility.  Apps will get only the
+ * events that they originally wanted.  Be sure to add new events here!
+ */
+#define IN_ALL_EVENTS	(IN_ACCESS | IN_MODIFY | IN_ATTRIB | IN_CLOSE_WRITE | \
+			 IN_CLOSE_NOWRITE | IN_OPEN | IN_MOVED_FROM | \
+			 IN_MOVED_TO | IN_DELETE | IN_CREATE | IN_DELETE_SELF)
+
+#define INOTIFY_IOCTL_MAGIC	'Q'
+#define INOTIFY_IOCTL_MAXNR	2
+
+#define INOTIFY_WATCH  		_IOR(INOTIFY_IOCTL_MAGIC, 1, struct inotify_watch_request)
+#define INOTIFY_IGNORE 		_IOR(INOTIFY_IOCTL_MAGIC, 2, int)
+
+#ifdef __KERNEL__
+
+#include <linux/dcache.h>
+#include <linux/fs.h>
+#include <linux/config.h>
+
+#ifdef CONFIG_INOTIFY
+
+extern void inotify_inode_queue_event(struct inode *, __u32, __u32,
+				      const char *);
+extern void inotify_dentry_parent_queue_event(struct dentry *, __u32, __u32,
+					      const char *);
+extern void inotify_unmount_inodes(struct list_head *);
+extern void inotify_inode_is_dead(struct inode *);
+extern u32 inotify_get_cookie(void);
+
+#else
+
+static inline void inotify_inode_queue_event(struct inode *inode,
+					     __u32 mask, __u32 cookie,
+					     const char *filename)
+{
+}
+
+static inline void inotify_dentry_parent_queue_event(struct dentry *dentry,
+						     __u32 mask, __u32 cookie,
+						     const char *filename)
+{
+}
+
+static inline void inotify_unmount_inodes(struct list_head *list)
+{
+}
+
+static inline void inotify_inode_is_dead(struct inode *inode)
+{
+}
+
+static inline u32 inotify_get_cookie(void)
+{
+	return 0;
+}
+
+#endif	/* CONFIG_INOTIFY */
+
+#endif	/* __KERNEL __ */
+
+#endif	/* _LINUX_INOTIFY_H */
diff -urN linux-2.6.13-rc1/include/linux/sched.h linux/include/linux/sched.h
--- linux-2.6.13-rc1/include/linux/sched.h	2005-07-01 16:12:14.000000000 -0400
+++ linux/include/linux/sched.h	2005-07-01 16:13:14.000000000 -0400
@@ -410,6 +410,10 @@
 	atomic_t processes;	/* How many processes does this user have? */
 	atomic_t files;		/* How many open files does this user have? */
 	atomic_t sigpending;	/* How many pending signals does this user have? */
+#ifdef CONFIG_INOTIFY
+	atomic_t inotify_watches; /* How many inotify watches does this user have? */
+	atomic_t inotify_devs;	/* How many inotify devs does this user have opened? */
+#endif
 	/* protected by mq_lock	*/
 	unsigned long mq_bytes;	/* How many bytes can be allocated to mqueue? */
 	unsigned long locked_shm; /* How many pages of mlocked shm ? */
diff -urN linux-2.6.13-rc1/kernel/user.c linux/kernel/user.c
--- linux-2.6.13-rc1/kernel/user.c	2005-07-01 16:10:51.000000000 -0400
+++ linux/kernel/user.c	2005-07-01 16:13:14.000000000 -0400
@@ -120,6 +120,10 @@
 		atomic_set(&new->processes, 0);
 		atomic_set(&new->files, 0);
 		atomic_set(&new->sigpending, 0);
+#ifdef CONFIG_INOTIFY
+		atomic_set(&new->inotify_watches, 0);
+		atomic_set(&new->inotify_devs, 0);
+#endif
 
 		new->mq_bytes = 0;
 		new->locked_shm = 0;

Re: Problem with inotify

[David Gómez]

Hi Robert,

On Jul 01 at 04:25:15, Robert Love wrote:
> > either introduced by inotify or just made more likely (i.e. 100% of time) 
> > by inotify but the change is elsewhere.
> 
> The mount case is baffling.  Inotify has no interaction whatsoever with
> mounting.  Unmounting, definitely--see fs/inotify.c ::
> inotify_unmount_inodes().

Good news are that it seems unrelated to inotify. I updated to 2.6.13-rc1, and
tested it with and without the inotify patch. Now I get the mount process
hanged in inode_wait with the vanilla kernel, and working perfectly with the
patched one. Weird...

> [ Attached is the latest inotify, against 2.6.13-rc1. ]

I tested it with this patch.

bye

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: Problem with inotify

[Robert Love]

On Sat, 2005-07-02 at 14:54 +0200, David Gómez wrote:
> Hi Robert,
> 
> On Jul 01 at 04:25:15, Robert Love wrote:
> > > either introduced by inotify or just made more likely (i.e. 100% of time) 
> > > by inotify but the change is elsewhere.
> > 
> > The mount case is baffling.  Inotify has no interaction whatsoever with
> > mounting.  Unmounting, definitely--see fs/inotify.c ::
> > inotify_unmount_inodes().
> 
> Good news are that it seems unrelated to inotify. I updated to 2.6.13-rc1, and
> tested it with and without the inotify patch. Now I get the mount process
> hanged in inode_wait with the vanilla kernel, and working perfectly with the
> patched one. Weird...

Sorry to hear that your problem continues, but glad to hear that it is
not inotify.  I was baffled by a lockup in mount; it made no sense.

Good luck finding a fix!

	Robert Love

Re: Problem with inotify

[Daniel Drake]

Hi Anton,

I'm trying to work around the NTFS lockup issue. Like others, I can reproduce
it just by opening nautilus on an NTFS partition (i.e. creating an inotify
watch on it) and then unmount at some point after - instant system lockup.

I have tried applying two patches:
fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation.patch from 2.6.13-rc1-mm1
and the "NTFS: Fix a nasty deadlock that appeared in recent kernels" patch
from your git tree.

However, I still get the freezing up on unmount. I am using 2.6.12, plus
inotify-0.23-15, and the two patches mentioned above. Anything else I can try?

Thanks,
Daniel

Re: Problem with inotify

[Anton Altaparmakov]

Hi,

On Sat, 2 Jul 2005, Daniel Drake wrote:
> I'm trying to work around the NTFS lockup issue. Like others, I can reproduce
> it just by opening nautilus on an NTFS partition (i.e. creating an inotify
> watch on it) and then unmount at some point after - instant system lockup.
> 
> I have tried applying two patches:
> fix-soft-lockup-due-to-ntfs-vfs-part-and-explanation.patch from 2.6.13-rc1-mm1
> and the "NTFS: Fix a nasty deadlock that appeared in recent kernels" patch
> from your git tree.
> 
> However, I still get the freezing up on unmount. I am using 2.6.12, plus
> inotify-0.23-15, and the two patches mentioned above. Anything else I can try?

Thinking about it some more made me realize that there may be a problem in 
inotify after all...  Could you try the below patch to fs/inotify.c and 
tell me if it cures the lockup you are seeing?  (Note patch compiles but 
is otherwise untested.  But given it locks up without the patch it can't 
do much worse with it!)

Thanks a lot in advance!

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

inotify_unmount_inodes-list-iteration-fix.diff

Patch description: I believe that the inode reference that is being 
dropped by inotify's remove_watch() can cause inodes other than the 
current @inode to be moved away from the per-sb list.  And if this happens 
to be the next inode in the list, i.e. @next_i, then the iteration will 
proceed on the list that @next_i was moved to rather than the per-sb list.  
Thus, the check in the for loop (list_for_each_entry_safe()) for the @head 
being reached will _never_ be true and hence the for loop will keep going 
for ever...  Even worse the memory backing @next_i could be completely 
freed and then completely random results would be obtained.

Basically, I do not believe that using list_for_each_entry_safe() is safe 
at all as it only guards against removal of the current entry but not 
against removal of the next entry.  This patch tries to work around this 
by getting a reference to @next_i whilst the inode_lock is dropped.

Signed-off-by: Anton Altaparmakov <aia21@cantab.net>

--- linux-2.6.13-rc1-mm1-vanilla/fs/inotify.c	2005-07-01 14:51:09.000000000 +0100
+++ linux-2.6.13-rc1-mm1/fs/inotify.c	2005-07-02 22:11:11.000000000 +0100
@@ -560,9 +560,10 @@ EXPORT_SYMBOL_GPL(inotify_get_cookie);
  */
 void inotify_unmount_inodes(struct list_head *list)
 {
-	struct inode *inode, *next_i;
+	struct inode *inode, *next_i, *need_iput = NULL;
 
 	list_for_each_entry_safe(inode, next_i, list, i_sb_list) {
+		struct inode *need_iput_tmp;
 		struct inotify_watch *watch, *next_w;
 		struct list_head *watches;
 
@@ -574,8 +575,20 @@ void inotify_unmount_inodes(struct list_
 		if (inode->i_state & (I_CLEAR | I_FREEING))
 			continue;
 
+		need_iput_tmp = need_iput;
+		need_iput = NULL;
+
 		/* In case the remove_watch() drops a reference */
-		__iget(inode);
+		if (inode != need_iput_tmp)
+			__iget(inode);
+		else
+			need_iput_tmp = NULL;
+
+		/* In case the dropping of a reference would nuke next_i. */
+		if (!next_i->i_state & (I_CLEAR | I_FREEING)) {
+			__iget(next_i);
+			need_iput = next_i;
+		}
 
 		/*
 		 * We can safely drop inode_lock here because the per-sb list
@@ -584,6 +597,9 @@ void inotify_unmount_inodes(struct list_
 		 */
 		spin_unlock(&inode_lock);
 
+		if (need_iput_tmp)
+			iput(need_iput_tmp);
+
 		/* for each watch, send IN_UNMOUNT and then remove it */
 		down(&inode->inotify_sem);
 		watches = &inode->inotify_watches;
@@ -599,6 +615,11 @@ void inotify_unmount_inodes(struct list_
 
 		spin_lock(&inode_lock);
 	}
+	if (need_iput) {
+		spin_unlock(&inode_lock);
+		iput(need_iput);
+		spin_lock(&inode_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(inotify_unmount_inodes);
 

Re: Problem with inotify

[Daniel Drake]

Hi Anton,

Anton Altaparmakov wrote:
> Thinking about it some more made me realize that there may be a problem in 
> inotify after all...  Could you try the below patch to fs/inotify.c and 
> tell me if it cures the lockup you are seeing?  (Note patch compiles but 
> is otherwise untested.  But given it locks up without the patch it can't 
> do much worse with it!)

Thanks for writing that patch, the effort is much appreciated. Unfortunately
it does not help :(

I've done a bit more investigating for you though. I did some tests purely on
the console, using inotify-test (from inotify-utils) which is the most
simplistic way you can use inotify: it just prints out the recieved events to
the screen.

I found out that unmount works perfectly well as long as there are no active
inotify watches (this might be quite obvious though!) - i.e. closing
inotify-test before unmounting results in a clean unmount. Unmounting while
inotify-test is watching the NTFS partition causes the freeze.

When the machine freezes, it still responds to ping, but not to ssh. Sysrq
works, so I got a sysrq-p trace:

Pid 8997 comm umount
EIP is at inotify_unmount_inodes+0x38/0x140

stack trace:
invalidate_inodes+0x40/0x90
generic_shutdown_super+0x59/0x140
kill_block_super+0x2d/0x50
deactivate_super+0x5a/0x90
sys_umount+0x3f/0x90
filp_close+0x52/0xa0
sys_oldumount+0x17/0x20
sysenter_past_esp+0x54/0x75

Investigating that function:

(gdb) list *inotify_unmount_inodes+0x38
0x9c8 is in inotify_unmount_inodes (inotify.c:565).
560      */
561     void inotify_unmount_inodes(struct list_head *list)
562     {
563             struct inode *inode, *next_i, *need_iput = NULL;
564
565             list_for_each_entry_safe(inode, next_i, list, i_sb_list) {
566                     struct inode *need_iput_tmp;
567                     struct inotify_watch *watch, *next_w;
568                     struct list_head *watches;
569

I then added a loop counter printk in at line 569 above. It shows that the
loop iterates 8 times on a clean unmount, and goes into a seemingly infinite
loop (i.e. freeze) when unmounting with inotify watches active.

I don't know much about filesystem internals so I'm pretty stuck here, haven't
reached that chapter of Robert's book yet ;)

Please let me know if theres any other info I can provide.

Thanks,
Daniel

Re: Problem with inotify

[Anton Altaparmakov]

Hi Daniel,

On Sun, 3 Jul 2005, Daniel Drake wrote:
> Anton Altaparmakov wrote:
> > Thinking about it some more made me realize that there may be a problem in 
> > inotify after all...  Could you try the below patch to fs/inotify.c and 
> > tell me if it cures the lockup you are seeing?  (Note patch compiles but 
> > is otherwise untested.  But given it locks up without the patch it can't 
> > do much worse with it!)
> 
> Thanks for writing that patch, the effort is much appreciated. Unfortunately
> it does not help :(

)-:

> I've done a bit more investigating for you though. I did some tests purely on
> the console, using inotify-test (from inotify-utils) which is the most
> simplistic way you can use inotify: it just prints out the recieved events to
> the screen.
> 
> I found out that unmount works perfectly well as long as there are no active
> inotify watches (this might be quite obvious though!) - i.e. closing
> inotify-test before unmounting results in a clean unmount. Unmounting while
> inotify-test is watching the NTFS partition causes the freeze.

Great stuff.  Thanks!  At least my patch appears to have been on the right 
track!  My analysis was that there is an infinite loop and this is what 
you are observing.  (-:  It is very interesting that the infinite loop 
only happens when actual watches are present.  It is 1am here so I am 
going to bed but I will think about this tomorrow/Monday and hopefully 
cook up a new patch which if you could test it would be great.

> When the machine freezes, it still responds to ping, but not to ssh. Sysrq
> works, so I got a sysrq-p trace:
> 
> Pid 8997 comm umount
> EIP is at inotify_unmount_inodes+0x38/0x140
> 
> stack trace:
> invalidate_inodes+0x40/0x90
> generic_shutdown_super+0x59/0x140
> kill_block_super+0x2d/0x50
> deactivate_super+0x5a/0x90
> sys_umount+0x3f/0x90
> filp_close+0x52/0xa0
> sys_oldumount+0x17/0x20
> sysenter_past_esp+0x54/0x75
> 
> Investigating that function:
> 
> (gdb) list *inotify_unmount_inodes+0x38
> 0x9c8 is in inotify_unmount_inodes (inotify.c:565).
> 560      */
> 561     void inotify_unmount_inodes(struct list_head *list)
> 562     {
> 563             struct inode *inode, *next_i, *need_iput = NULL;
> 564
> 565             list_for_each_entry_safe(inode, next_i, list, i_sb_list) {
> 566                     struct inode *need_iput_tmp;
> 567                     struct inotify_watch *watch, *next_w;
> 568                     struct list_head *watches;
> 569
> 
> I then added a loop counter printk in at line 569 above. It shows that the
> loop iterates 8 times on a clean unmount, and goes into a seemingly infinite
> loop (i.e. freeze) when unmounting with inotify watches active.

Cool.  Thanks for that.  Could you modify your printk to output:

printk(KERN_ERR "doing sb 0x%lx, inode 0x%lx, i_state 0x%x\n", 
(unsigned long)inode->i_sb, inode->i_ino, inode->i_state);

Eeek!  I just reread my patch.  I am a muppet!  Instead of the printk (or 
in addition if you like), please try the corrected version below.  The 
original version completely fails to do anything at all.  It's amazing 
what the omission of a single parenthesis pair can do to your code logic.  
)-:

Thanks a lot in advance and many apologies for wasting your time with a 
bogus patch!

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

inotify_unmount_inodes-list-iteration-fix2.diff

Patch description: I believe that the inode reference that is being 
dropped by inotify's remove_watch() can cause inodes other than the 
current @inode to be moved away from the per-sb list.  And if this happens 
to be the next inode in the list, i.e. @next_i, then the iteration will 
proceed on the list that @next_i was moved to rather than the per-sb list.  
Thus, the check in the for loop (list_for_each_entry_safe()) for the @head 
being reached will _never_ be true and hence the for loop will keep going 
for ever...  Even worse the memory backing @next_i could be completely 
freed and then completely random results would be obtained.

Basically, I do not believe that using list_for_each_entry_safe() is safe 
at all as it only guards against removal of the current entry but not 
against removal of the next entry.  This patch tries to work around this 
by getting a reference to @next_i whilst the inode_lock is dropped.

Signed-off-by: Anton Altaparmakov <aia21@cantab.net>

--- linux-2.6.13-rc1-mm1-vanilla/fs/inotify.c	2005-07-01 14:51:09.000000000 +0100
+++ linux-2.6.13-rc1-mm1/fs/inotify.c	2005-07-02 22:11:11.000000000 +0100
@@ -560,9 +560,10 @@ EXPORT_SYMBOL_GPL(inotify_get_cookie);
  */
 void inotify_unmount_inodes(struct list_head *list)
 {
-	struct inode *inode, *next_i;
+	struct inode *inode, *next_i, *need_iput = NULL;
 
 	list_for_each_entry_safe(inode, next_i, list, i_sb_list) {
+		struct inode *need_iput_tmp;
 		struct inotify_watch *watch, *next_w;
 		struct list_head *watches;
 
@@ -574,8 +575,20 @@ void inotify_unmount_inodes(struct list_
 		if (inode->i_state & (I_CLEAR | I_FREEING))
 			continue;
 
+		need_iput_tmp = need_iput;
+		need_iput = NULL;
+
 		/* In case the remove_watch() drops a reference */
-		__iget(inode);
+		if (inode != need_iput_tmp)
+			__iget(inode);
+		else
+			need_iput_tmp = NULL;
+
+		/* In case the dropping of a reference would nuke next_i. */
+		if (!(next_i->i_state & (I_CLEAR | I_FREEING))) {
+			__iget(next_i);
+			need_iput = next_i;
+		}
 
 		/*
 		 * We can safely drop inode_lock here because the per-sb list
@@ -584,6 +597,9 @@ void inotify_unmount_inodes(struct list_
 		 */
 		spin_unlock(&inode_lock);
 
+		if (need_iput_tmp)
+			iput(need_iput_tmp);
+
 		/* for each watch, send IN_UNMOUNT and then remove it */
 		down(&inode->inotify_sem);
 		watches = &inode->inotify_watches;
@@ -599,6 +615,11 @@ void inotify_unmount_inodes(struct list_
 
 		spin_lock(&inode_lock);
 	}
+	if (need_iput) {
+		spin_unlock(&inode_lock);
+		iput(need_iput);
+		spin_lock(&inode_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(inotify_unmount_inodes);
 

Re: Problem with inotify

[Daniel Drake]

Anton Altaparmakov wrote:
> Eeek!  I just reread my patch.  I am a muppet!  Instead of the printk (or 
> in addition if you like), please try the corrected version below.  The 
> original version completely fails to do anything at all.  It's amazing 
> what the omission of a single parenthesis pair can do to your code logic.  
> )-:

I reverted the patch you sent earlier
(inotify_unmount_inodes-list-iteration-fix.diff) and applied the one you
attached here (inotify_unmount_inodes-list-iteration-fix2.diff).

The good news is that the hang is gone. The bad news is that you cured the
hang by introducing an oops :(

Unable to handle kernel NULL pointer dereference at virtual address 00000024
 printing eip:
c01751d9
*pde = 00000000
Oops: 0000 [#1]
PREEMPT
Modules linked in: ntfs snd_pcm_oss snd_mixer_oss snd_seq_oss
snd_seq_midi_event snd_seq snd_seq_device snd_intel8x0 snd_ac97_codec snd_pcm
snd_timer snd snd_page_alloc
CPU:    0
EIP:    0060:[<c01751d9>]    Tainted: P      VLI
EFLAGS: 00010283   (2.6.12-gentoo-r3)
EIP is at iput+0x19/0x90
eax: 00000000   ebx: deb82e64   ecx: deb82e64   edx: deb82e64
esi: dd01ff00   edi: deb82e64   ebp: dd01ff00   esp: df7b9eb8
ds: 007b   es: 007b   ss: 0068
Process umount (pid: 9201, threadinfo=df7b8000 task=dd656a60)
Stack: dd01fe20 c03feb20 df7b8000 c0182337 deb82e64 00000000 00000000 00000000
       00000000 df7b8000 dd01ff08 deb82e64 dd01f0dc df7b8000 deb82e74 df7b9f08
       df7b8000 c0174050 deb82e74 dffe0040 df7b9f08 df7b9f08 dda12274 deb82e00
Call Trace:
 [<c0182337>] inotify_unmount_inodes+0x187/0x1e0
 [<c0174050>] invalidate_inodes+0x40/0x90
 [<c015f579>] generic_shutdown_super+0x59/0x140
 [<c01601dd>] kill_block_super+0x2d/0x50
 [<c015f40a>] deactivate_super+0x5a/0x90
 [<c01773cf>] sys_umount+0x3f/0x90
 [<c0157e42>] filp_close+0x52/0xa0
 [<c0177437>] sys_oldumount+0x17/0x20
 [<c010326b>] sysenter_past_esp+0x54/0x75
Code: ff ff 89 44 24 04 e9 47 fe ff ff 8d b4 26 00 00 00 00 53 83 ec 08 8b 5c
24 10 85 db 74 58 83 bb 24 01 00 00 20 8b 83 94 00 00 00 <8b> 40 24 74 5a 85
c0 74 07 8b 50 14 85 d2 75 47 8d 43 24 c7 44

(gdb) list *iput+0x19
0x1789 is in iput (inode.c:1131).
1126     *      Consequently, iput() can sleep.
1127     */
1128    void iput(struct inode *inode)
1129    {
1130            if (inode) {
1131                    struct super_operations *op = inode->i_sb->s_op;
1132
1133                    BUG_ON(inode->i_state == I_CLEAR);
1134
1135                    if (op && op->put_inode)


Note that this exact oops happens even when inotify has not been used, i.e. to
reproduce this oops, I can just do:

mount /mnt/ntfs ; umount /mnt/ntfs

What next? :)

Thanks,
Daniel

Re: Problem with inotify

[Anton Altaparmakov]

On Sun, 2005-07-03 at 11:34 +0100, Daniel Drake wrote:
> I reverted the patch you sent earlier
> (inotify_unmount_inodes-list-iteration-fix.diff) and applied the one you
> attached here (inotify_unmount_inodes-list-iteration-fix2.diff).
> 
> The good news is that the hang is gone. The bad news is that you cured the
> hang by introducing an oops :(

)-:  I have addressed the only things I can think off that could cause
the oops and below is the resulting patch.  Could you please test it?

Thanks a lot!

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

--- linux-2.6.13-rc1-mm1-vanilla/fs/inotify.c	2005-07-01 14:51:09.000000000 +0100
+++ linux-2.6.13-rc1-mm1/fs/inotify.c	2005-07-04 15:18:14.000000000 +0100
@@ -560,22 +560,45 @@ EXPORT_SYMBOL_GPL(inotify_get_cookie);
  */
 void inotify_unmount_inodes(struct list_head *list)
 {
-	struct inode *inode, *next_i;
+	struct inode *inode, *next_i, *need_iput = NULL;
 
 	list_for_each_entry_safe(inode, next_i, list, i_sb_list) {
+		struct inode *need_iput_tmp;
 		struct inotify_watch *watch, *next_w;
 		struct list_head *watches;
 
 		/*
+		 * If i_count is zero, the inode cannot have any watches and
+		 * doing an __iget/iput with MS_ACTIVE clear would actually
+		 * evict all inodes from icache which is unnecessarily violent.
+		 */
+		if (!atomic_read(&inode->i_count))
+			continue;
+		/*
 		 * We cannot __iget() an inode in state I_CLEAR or I_FREEING,
 		 * which is fine becayse by that point the inode cannot have
 		 * any associated watches.
 		 */
-		if (inode->i_state & (I_CLEAR | I_FREEING))
+		if (inode->i_state & (I_CLEAR | I_FREEING | I_WILL_FREE))
 			continue;
 
+		need_iput_tmp = need_iput;
+		need_iput = NULL;
+
 		/* In case the remove_watch() drops a reference */
-		__iget(inode);
+		if (inode != need_iput_tmp)
+			__iget(inode);
+		else
+			need_iput_tmp = NULL;
+
+		/* In case the dropping of a reference would nuke next_i. */
+		if ((&next_i->i_sb_list != list) &&
+				atomic_read(&next_i->i_count) &&
+				!(next_i->i_state & (I_CLEAR | I_FREEING |
+				I_WILL_FREE))) {
+			__iget(next_i);
+			need_iput = next_i;
+		}
 
 		/*
 		 * We can safely drop inode_lock here because the per-sb list
@@ -584,6 +607,9 @@ void inotify_unmount_inodes(struct list_
 		 */
 		spin_unlock(&inode_lock);
 
+		if (need_iput_tmp)
+			iput(need_iput_tmp);
+
 		/* for each watch, send IN_UNMOUNT and then remove it */
 		down(&inode->inotify_sem);
 		watches = &inode->inotify_watches;

Re: Problem with inotify

[Anton Altaparmakov]

On Mon, 2005-07-04 at 15:27 +0100, Anton Altaparmakov wrote:
> On Sun, 2005-07-03 at 11:34 +0100, Daniel Drake wrote:
> > I reverted the patch you sent earlier
> > (inotify_unmount_inodes-list-iteration-fix.diff) and applied the one you
> > attached here (inotify_unmount_inodes-list-iteration-fix2.diff).
> > 
> > The good news is that the hang is gone. The bad news is that you cured the
> > hang by introducing an oops :(
> 
> )-:  I have addressed the only things I can think off that could cause
> the oops and below is the resulting patch.  Could you please test it?

I forgot to say that this patch is a replacement for the previous
(inotify_unmount_inodes-list-iteration-fix{,2}.diff}.

Best regards,

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: Problem with inotify

[Anton Altaparmakov]

On Mon, 2005-07-04 at 15:39 +0100, Anton Altaparmakov wrote:
> On Mon, 2005-07-04 at 15:27 +0100, Anton Altaparmakov wrote:
> > On Sun, 2005-07-03 at 11:34 +0100, Daniel Drake wrote:
> > > I reverted the patch you sent earlier
> > > (inotify_unmount_inodes-list-iteration-fix.diff) and applied the one you
> > > attached here (inotify_unmount_inodes-list-iteration-fix2.diff).
> > > 
> > > The good news is that the hang is gone. The bad news is that you cured the
> > > hang by introducing an oops :(
> > 
> > )-:  I have addressed the only things I can think off that could cause
> > the oops and below is the resulting patch.  Could you please test it?
> 
> I forgot to say that this patch is a replacement for the previous
> (inotify_unmount_inodes-list-iteration-fix{,2}.diff}.

Note I seem to remember you are using 2.6.12?  If so I am not sure when
the I_WILL_FREE stuff came into existence, so should inotify.c not
compile after applying this patch just delete the " | I_WILL_FREE " in
the two places it occurs introduced by the patch and all should be fine.

Best regards,

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

RE: Problem with inotify

[Gautam Singaraju]

Anton,

I had used the 2.6.12 kernel with the latest Inotify. There was no
"I_WILL_FREE" in the any place. And, there was no problem in compilation.

I believe Inotify is very useful and should be included in the next versions
of the kernel. Are there any ongoing plans for this?
 
Thanks,
Gautam Singaraju
SIS Dept., UNC Charlotte

-----Original Message-----
From: linux-kernel-owner@vger.kernel.org
[mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Anton Altaparmakov
Sent: Monday, July 04, 2005 11:12 AM
To: Daniel Drake
Cc: David Gómez; Robert Love; John McCutchan; Linux-kernel
Subject: Re: Problem with inotify

On Mon, 2005-07-04 at 15:39 +0100, Anton Altaparmakov wrote:
> On Mon, 2005-07-04 at 15:27 +0100, Anton Altaparmakov wrote:
> > On Sun, 2005-07-03 at 11:34 +0100, Daniel Drake wrote:
> > > I reverted the patch you sent earlier
> > > (inotify_unmount_inodes-list-iteration-fix.diff) and applied the one
you
> > > attached here (inotify_unmount_inodes-list-iteration-fix2.diff).
> > > 
> > > The good news is that the hang is gone. The bad news is that you cured
the
> > > hang by introducing an oops :(
> > 
> > )-:  I have addressed the only things I can think off that could cause
> > the oops and below is the resulting patch.  Could you please test it?
> 
> I forgot to say that this patch is a replacement for the previous
> (inotify_unmount_inodes-list-iteration-fix{,2}.diff}.

Note I seem to remember you are using 2.6.12?  If so I am not sure when
the I_WILL_FREE stuff came into existence, so should inotify.c not
compile after applying this patch just delete the " | I_WILL_FREE " in
the two places it occurs introduced by the patch and all should be fine.

Best regards,

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: Problem with inotify

[Anton Altaparmakov]

Gautam,

On Mon, 2005-07-04 at 11:55 -0400, Gautam Singaraju wrote:
> I had used the 2.6.12 kernel with the latest Inotify. There was no
> "I_WILL_FREE" in the any place. And, there was no problem in compilation.

Er, yes, obviously.  You are not using my patch on top of inotify and
original inotify does not use I_WILL_FREE.  You are answering somewhat
out of context...

> I believe Inotify is very useful and should be included in the next versions
> of the kernel. Are there any ongoing plans for this?

I cannot answer this.  Robert/John?  However there appear to be issues
with it at the moment (the umount problems with ntfs volumes) so
probably not quite yet unless we figure out the problems...

Best regards,

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: Problem with inotify

[Daniel Drake]

Anton Altaparmakov wrote:
> )-:  I have addressed the only things I can think off that could cause
> the oops and below is the resulting patch.  Could you please test it?

Yeah!! After removing I_WILL_FREE stuff, that fixed both the oops *and* the
hang. Everything works nicely now.

Thanks a million!

Daniel

Re: Problem with inotify

[Anton Altaparmakov]

On Mon, 4 Jul 2005, Daniel Drake wrote:
> Anton Altaparmakov wrote:
> > )-:  I have addressed the only things I can think off that could cause
> > the oops and below is the resulting patch.  Could you please test it?
> 
> Yeah!! After removing I_WILL_FREE stuff, that fixed both the oops *and* the
> hang. Everything works nicely now.

Great!  Thanks a lot for testing!  I will send a patch to Robert and 
Andrew in a minute with more comments added.

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: Problem with inotify

[John McCutchan]

On Mon, 2005-07-04 at 20:09 +0100, Anton Altaparmakov wrote:
> On Mon, 4 Jul 2005, Daniel Drake wrote:
> > Anton Altaparmakov wrote:
> > > )-:  I have addressed the only things I can think off that could cause
> > > the oops and below is the resulting patch.  Could you please test it?
> > 
> > Yeah!! After removing I_WILL_FREE stuff, that fixed both the oops *and* the
> > hang. Everything works nicely now.
> 
> Great!  Thanks a lot for testing!  I will send a patch to Robert and 
> Andrew in a minute with more comments added.

Nice work, I am going to have a closer look at the patch soon. Could you
post the final patch at http://bugzilla.kernel.org/show_bug.cgi?id=4796

Thanks,
-- 
John McCutchan <ttb@tentacle.dhs.org>

Re: Problem with inotify

[Anton Altaparmakov]

On Mon, 4 Jul 2005, John McCutchan wrote:
> On Mon, 2005-07-04 at 20:09 +0100, Anton Altaparmakov wrote:
> > On Mon, 4 Jul 2005, Daniel Drake wrote:
> > > Anton Altaparmakov wrote:
> > > > )-:  I have addressed the only things I can think off that could cause
> > > > the oops and below is the resulting patch.  Could you please test it?
> > > 
> > > Yeah!! After removing I_WILL_FREE stuff, that fixed both the oops *and* the
> > > hang. Everything works nicely now.
> > 
> > Great!  Thanks a lot for testing!  I will send a patch to Robert and 
> > Andrew in a minute with more comments added.
> 
> Nice work, I am going to have a closer look at the patch soon. Could you
> post the final patch at http://bugzilla.kernel.org/show_bug.cgi?id=4796

Thanks.  Now done.  But I am not sure about the white space.  I can't get 
anything sensible out of IE on Mac OS 9 which I am on at the moment.

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: Problem with inotify

[John McCutchan]

On Tue, Jul 05, 2005 at 08:56:15AM +0100, Anton Altaparmakov wrote:
> On Mon, 4 Jul 2005, John McCutchan wrote:
> > On Mon, 2005-07-04 at 20:09 +0100, Anton Altaparmakov wrote:
> > > On Mon, 4 Jul 2005, Daniel Drake wrote:
> > > > Anton Altaparmakov wrote:
> > > > > )-:  I have addressed the only things I can think off that could cause
> > > > > the oops and below is the resulting patch.  Could you please test it?
> > > > 
> > > > Yeah!! After removing I_WILL_FREE stuff, that fixed both the oops *and* the
> > > > hang. Everything works nicely now.
> > > 
> > > Great!  Thanks a lot for testing!  I will send a patch to Robert and 
> > > Andrew in a minute with more comments added.
> > 
> > Nice work, I am going to have a closer look at the patch soon. Could you
> > post the final patch at http://bugzilla.kernel.org/show_bug.cgi?id=4796
> 
> Thanks.  Now done.  But I am not sure about the white space.  I can't get 
> anything sensible out of IE on Mac OS 9 which I am on at the moment.
> 

Originally inotify had 2 functions that handled this. One that would
build up a list of inodes to call remove_watch on, the other function
would do the actual calling of remove_watch. This mirrored the other
unmount paths. I'm wondering if it wouldn't be cleaner to revert back to
that way?

John

Re: Problem with inotify

[Anton Altaparmakov]

On Tue, 5 Jul 2005, John McCutchan wrote:
> On Tue, Jul 05, 2005 at 08:56:15AM +0100, Anton Altaparmakov wrote:
> > On Mon, 4 Jul 2005, John McCutchan wrote:
> > > On Mon, 2005-07-04 at 20:09 +0100, Anton Altaparmakov wrote:
> > > > On Mon, 4 Jul 2005, Daniel Drake wrote:
> > > > > Anton Altaparmakov wrote:
> > > > > > )-:  I have addressed the only things I can think off that could cause
> > > > > > the oops and below is the resulting patch.  Could you please test it?
> > > > > 
> > > > > Yeah!! After removing I_WILL_FREE stuff, that fixed both the oops *and* the
> > > > > hang. Everything works nicely now.
> > > > 
> > > > Great!  Thanks a lot for testing!  I will send a patch to Robert and 
> > > > Andrew in a minute with more comments added.
> > > 
> > > Nice work, I am going to have a closer look at the patch soon. Could you
> > > post the final patch at http://bugzilla.kernel.org/show_bug.cgi?id=4796
> > 
> > Thanks.  Now done.  But I am not sure about the white space.  I can't get 
> > anything sensible out of IE on Mac OS 9 which I am on at the moment.
> 
> Originally inotify had 2 functions that handled this. One that would
> build up a list of inodes to call remove_watch on, the other function
> would do the actual calling of remove_watch. This mirrored the other
> unmount paths. I'm wondering if it wouldn't be cleaner to revert back to
> that way?

That is certainly possible.  Out of curiosity, how did you anchor the 
inodes to your private list?  Or did you just have a dynamically allocated 
array of pointers to inodes?

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: Problem with inotify

[John McCutchan]

On Tue, Jul 05, 2005 at 06:06:56PM +0100, Anton Altaparmakov wrote:
> On Tue, 5 Jul 2005, John McCutchan wrote:
> > On Tue, Jul 05, 2005 at 08:56:15AM +0100, Anton Altaparmakov wrote:
> > > On Mon, 4 Jul 2005, John McCutchan wrote:
> > > > On Mon, 2005-07-04 at 20:09 +0100, Anton Altaparmakov wrote:
> > > > > On Mon, 4 Jul 2005, Daniel Drake wrote:
> > > > > > Anton Altaparmakov wrote:
> > > > > > > )-:  I have addressed the only things I can think off that could cause
> > > > > > > the oops and below is the resulting patch.  Could you please test it?
> > > > > > 
> > > > > > Yeah!! After removing I_WILL_FREE stuff, that fixed both the oops *and* the
> > > > > > hang. Everything works nicely now.
> > > > > 
> > > > > Great!  Thanks a lot for testing!  I will send a patch to Robert and 
> > > > > Andrew in a minute with more comments added.
> > > > 
> > > > Nice work, I am going to have a closer look at the patch soon. Could you
> > > > post the final patch at http://bugzilla.kernel.org/show_bug.cgi?id=4796
> > > 
> > > Thanks.  Now done.  But I am not sure about the white space.  I can't get 
> > > anything sensible out of IE on Mac OS 9 which I am on at the moment.
> > 
> > Originally inotify had 2 functions that handled this. One that would
> > build up a list of inodes to call remove_watch on, the other function
> > would do the actual calling of remove_watch. This mirrored the other
> > unmount paths. I'm wondering if it wouldn't be cleaner to revert back to
> > that way?
> 
> That is certainly possible.  Out of curiosity, how did you anchor the 
> inodes to your private list?  Or did you just have a dynamically allocated 
> array of pointers to inodes?
> 

The only reason I suggested it, is I'm afraid that maybe we are still
missing a corner case. Even with your fix.

We just added a new list_head in the inode struct. If you look at older
versions of inotify, maybe around 0.15 you can see for yourself.

John

Re: Problem with inotify

[Anton Altaparmakov]

On Tue, 5 Jul 2005, John McCutchan wrote:
> On Tue, Jul 05, 2005 at 06:06:56PM +0100, Anton Altaparmakov wrote:
> > On Tue, 5 Jul 2005, John McCutchan wrote:
> > > On Mon, 4 Jul 2005, John McCutchan wrote:
> > > > Nice work, I am going to have a closer look at the patch soon. Could you
> > > > post the final patch at http://bugzilla.kernel.org/show_bug.cgi?id=4796
> > > 
> > > Originally inotify had 2 functions that handled this. One that would
> > > build up a list of inodes to call remove_watch on, the other function
> > > would do the actual calling of remove_watch. This mirrored the other
> > > unmount paths. I'm wondering if it wouldn't be cleaner to revert back to
> > > that way?
> > 
> > That is certainly possible.  Out of curiosity, how did you anchor the 
> > inodes to your private list?  Or did you just have a dynamically allocated 
> > array of pointers to inodes?
> 
> The only reason I suggested it, is I'm afraid that maybe we are still
> missing a corner case. Even with your fix.

Oh, are you thinking of anything in particular?  I don't think you are 
missing anything now...

You cannot get any new watches which makes things a lot easier (see 
analysis I just posted in the other thread on LKML: Re: [-mm patch] Fix 
inotify umount hangs).  Thus you can ignore anything that cannot possibly 
have a watch and that is what my patch does.  Both i_count of zero and the 
three i_states I_CLEAR, I_FREEING, and I_WILL_FREE, which imply an i_count 
of zero mean that no watches can be present since each watch holds a 
reference via i_count.

> We just added a new list_head in the inode struct. If you look at older
> versions of inotify, maybe around 0.15 you can see for yourself.

That is far too space wasting IMHO given you only use it at umount time.  
That means you are wasting 8 or 16 bytes for each and every inode on the 
system.  If you are going down that route, IMHO it would be better if you 
just do an array of inodes and then walk that.  For scalability you could 
use a two dimensional array of pages each holding inodes for example.  It 
doesn't really matter what you do given umount is not exactly a hot path 
so the overhead of doing the necessary page allocations and then freeing 
them is not important.

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/