Re: Problem with inotify

From: Anton Altaparmakov (aia21_at_cam.ac.uk)
Date: 07/04/05

  • Next message: Shawn Starr: "Re: IBM HDAPS things are looking up (was: Re: [Hdaps-devel] Re: [ltp] IBM HDAPS Someone interested? (Accelerometer))" 
    To: Daniel Drake <dsd@gentoo.org>
Date:	Mon, 04 Jul 2005 15:27:21 +0100

    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;
-
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/
  • Next message: Shawn Starr: "Re: IBM HDAPS things are looking up (was: Re: [Hdaps-devel] Re: [ltp] IBM HDAPS Someone interested? (Accelerometer))"

    Relevant Pages