[PATCH 0/14] fsnotify: simplify locking

[PATCH 0/14] fsnotify: simplify locking

[Lino Sanfilippo]

This patchset simplifies some of the locking that is needed to handle the addition and 
removal of marks from groups and vfs objects (inodes, mounts). It applies against
commit ef9bf3b7144bee6ce1da5616015cabc8771206af of branch 
'origin/for-next' from git.infradead.org/users/eparis/notify.git


The recent locking order used in fsnotify is 

mark->lock
group->mark_lock
inode->i_lock

The problem is that we cant use the group->mark_lock to synchronize addition and 
removal of marks to/from a group, so we have to use an additional mutex, namely the 
group->mutex, for this purpose. 
We also need a dedicated kernel thread for mark destruction since we cant call 
synchronize() for the mark_srcu with a spinlock held (at least i assume that this 
is the main reason this thread exists).

The main goal of these patches is to change the locking order to 

group->mark_lock
inode->i_lock
mark->lock

and by this simplify the concerning code:
- use the group->mark_lock for group list iteration and addition/removal of marks
- call synchronize() for mark_srcu as soon as a mark is removed from its 
  fsobject and thus avoid the need for the "fsnotify_mark" thread
- avoid temporary lists for mark traversal 

 

[PATCH 01/14] fsnotify: change locking order in fsnotify_add_mark()

[Lino Sanfilippo]

Instead of taking
1. mark->lock
2. group->mark_lock
3. inode->lock

we now use another order in fsnotify_add_mark():
1. inode->lock
2. mark->lock

and assume that the callers took the group->mark_lock already.
By taking the mark_lock first, we can protect modification of the groups mark
list without the need of an extra lock like the group->mutex.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/mark.c |   38 +++++---------------------------------
 1 files changed, 5 insertions(+), 33 deletions(-)

diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 28b64eb..a677af0 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -213,20 +213,9 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
 
 	/*
 	 * LOCKING ORDER!!!!
-	 * mark->lock
-	 * group->mark_lock
 	 * inode->i_lock
+	 * mark->lock
 	 */
-	spin_lock(&mark->lock);
-	spin_lock(&group->mark_lock);
-
-	mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
-	mark->group = group;
-	list_add(&mark->g_list, &group->marks_list);
-	fsnotify_get_mark(mark); /* for i_list and g_list */
-	atomic_inc(&group->num_marks);
-
-	ret = 0;
 	if (inode)
 		ret = fsnotify_add_inode_mark(mark, group, inode, allow_dups);
 	else if (mnt)
@@ -234,34 +223,17 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
 	else
 		BUG();
 	if (ret)
-		goto err;
-
-	spin_unlock(&group->mark_lock);
+		return ret;
 
-	/* this will pin the object if appropriate */
-	fsnotify_set_mark_mask_locked(mark, mark->mask);
+	fsnotify_get_mark(mark); /* for i_list and g_list */
 
-	spin_unlock(&mark->lock);
+	list_add(&mark->g_list, &group->marks_list);
+	atomic_inc(&group->num_marks);
 
 	if (inode)
 		__fsnotify_update_child_dentry_flags(inode);
 
 	return 0;
-err:
-	mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
-	list_del_init(&mark->g_list);
-	mark->group = NULL;
-	atomic_dec(&group->num_marks);
-
-	spin_unlock(&group->mark_lock);
-	spin_unlock(&mark->lock);
-
-	spin_lock(&destroy_lock);
-	list_add(&mark->destroy_list, &destroy_list);
-	spin_unlock(&destroy_lock);
-	wake_up(&destroy_waitq);
-
-	return ret;
 }
 
 /*
-- 
1.5.6.5

[PATCH 02/14] fsnotify: rename fsnotify_add_mark() to fsnotify_add_mark_locked()

[Lino Sanfilippo]

We assume that the caller of fsnotify_add_mark() has taken the group->mark_lock
itself, so we should rename the function to fsnotify_add_mark_locked().

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/dnotify/dnotify.c        |    3 ++-
 fs/notify/fanotify/fanotify_user.c |    4 ++--
 fs/notify/inotify/inotify_user.c   |    3 ++-
 fs/notify/mark.c                   |    7 ++++---
 include/linux/fsnotify_backend.h   |    6 ++++--
 kernel/audit_tree.c                |    8 +++++---
 kernel/audit_watch.c               |    3 ++-
 7 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 89ec7e0..a81e97f 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -333,7 +333,8 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
 		dn_mark = container_of(fsn_mark, struct dnotify_mark, fsn_mark);
 		spin_lock(&fsn_mark->lock);
 	} else {
-		fsnotify_add_mark(new_fsn_mark, dnotify_group, inode, NULL, 0);
+		fsnotify_add_mark_locked(new_fsn_mark, dnotify_group, inode,
+					 NULL, 0);
 		spin_lock(&new_fsn_mark->lock);
 		fsn_mark = new_fsn_mark;
 		dn_mark = new_dn_mark;
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 2d4925b..41bfdfd 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -638,7 +638,7 @@ static int fanotify_add_vfsmount_mark(struct fsnotify_group *group,
 			goto err;
 
 		fsnotify_init_mark(fsn_mark, fanotify_free_mark);
-		ret = fsnotify_add_mark(fsn_mark, group, NULL, mnt, 0);
+		ret = fsnotify_add_mark_locked(fsn_mark, group, NULL, mnt, 0);
 		if (ret)
 			goto err2;
 	}
@@ -688,7 +688,7 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,
 			goto err;
 
 		fsnotify_init_mark(fsn_mark, fanotify_free_mark);
-		ret = fsnotify_add_mark(fsn_mark, group, inode, NULL, 0);
+		ret = fsnotify_add_mark_locked(fsn_mark, group, inode, NULL, 0);
 		if (ret)
 			goto err2;
 	}
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 4cd5d5d..bb91b53 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -656,7 +656,8 @@ static int inotify_new_watch(struct fsnotify_group *group,
 		goto out_err;
 
 	/* we are on the idr, now get on the inode */
-	ret = fsnotify_add_mark(&tmp_i_mark->fsn_mark, group, inode, NULL, 0);
+	ret = fsnotify_add_mark_locked(&tmp_i_mark->fsn_mark, group, inode,
+				       NULL, 0);
 	if (ret) {
 		/* we failed to get on the inode, get off the idr */
 		inotify_remove_from_idr(group, tmp_i_mark);
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index a677af0..247cbdf 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -201,10 +201,11 @@ void fsnotify_set_mark_ignored_mask_locked(struct fsnotify_mark *mark, __u32 mas
  * Attach an initialized mark to a given group and fs object.
  * These marks may be used for the fsnotify backend to determine which
  * event types should be delivered to which group.
+ * Requires groups mark_lock to be held.
  */
-int fsnotify_add_mark(struct fsnotify_mark *mark,
-		      struct fsnotify_group *group, struct inode *inode,
-		      struct vfsmount *mnt, int allow_dups)
+int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
+			     struct fsnotify_group *group, struct inode *inode,
+			     struct vfsmount *mnt, int allow_dups)
 {
 	int ret;
 
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 6a3c660..b018e78 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -402,8 +402,10 @@ extern void fsnotify_set_mark_ignored_mask_locked(struct fsnotify_mark *mark, __
 /* set the mask of a mark (might pin the object into memory */
 extern void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask);
 /* attach the mark to both the group and the inode */
-extern int fsnotify_add_mark(struct fsnotify_mark *mark, struct fsnotify_group *group,
-			     struct inode *inode, struct vfsmount *mnt, int allow_dups);
+extern int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
+				    struct fsnotify_group *group,
+				    struct inode *inode, struct vfsmount *mnt,
+				    int allow_dups);
 /* given a mark, flag it to be freed when all references are dropped */
 extern void fsnotify_destroy_mark(struct fsnotify_mark *mark);
 /* run all the marks in a group, and clear all of the vfsmount marks */
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 37b2bea..4b50ccc 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -264,7 +264,8 @@ static void untag_chunk(struct node *p)
 		goto Fallback;
 
 	fsnotify_duplicate_mark(&new->mark, entry);
-	if (fsnotify_add_mark(&new->mark, new->mark.group, new->mark.i.inode, NULL, 1)) {
+	if (fsnotify_add_mark_locked(&new->mark, new->mark.group,
+				     new->mark.i.inode, NULL, 1)) {
 		free_chunk(new);
 		goto Fallback;
 	}
@@ -327,7 +328,7 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree)
 		return -ENOMEM;
 
 	entry = &chunk->mark;
-	if (fsnotify_add_mark(entry, audit_tree_group, inode, NULL, 0)) {
+	if (fsnotify_add_mark_locked(entry, audit_tree_group, inode, NULL, 0)) {
 		free_chunk(chunk);
 		return -ENOSPC;
 	}
@@ -400,7 +401,8 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
 	}
 
 	fsnotify_duplicate_mark(chunk_entry, old_entry);
-	if (fsnotify_add_mark(chunk_entry, chunk_entry->group, chunk_entry->i.inode, NULL, 1)) {
+	if (fsnotify_add_mark_locked(chunk_entry, chunk_entry->group,
+				     chunk_entry->i.inode, NULL, 1)) {
 		spin_unlock(&old_entry->lock);
 		free_chunk(chunk);
 		fsnotify_put_mark(old_entry);
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index d2e3c78..6a57231 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -158,7 +158,8 @@ static struct audit_parent *audit_init_parent(struct nameidata *ndp)
 
 	fsnotify_init_mark(&parent->mark, audit_watch_free_mark);
 	parent->mark.mask = AUDIT_FS_WATCH;
-	ret = fsnotify_add_mark(&parent->mark, audit_watch_group, inode, NULL, 0);
+	ret = fsnotify_add_mark_locked(&parent->mark, audit_watch_group, inode,
+				       NULL, 0);
 	if (ret < 0) {
 		audit_free_parent(parent);
 		return ERR_PTR(ret);
-- 
1.5.6.5

[PATCH 03/14] fsnotify: adjust locking in fsnotify_add_[inode|vfsmount]_mark()

[Lino Sanfilippo]

Adjust fsnotify_add_[inode|vfsmount]_mark() to new locking order by taking the
inode/mount lock before the mark lock is taken. We also set all mark flags (after
we took the mark lock) in this function.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/inode_mark.c    |   24 +++++++++++++++++++-----
 fs/notify/vfsmount_mark.c |   19 ++++++++++++++++---
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 4c29fcf..8d35a84 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -188,14 +188,17 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark,
 	struct hlist_node *node, *last = NULL;
 	int ret = 0;
 
-	mark->flags |= FSNOTIFY_MARK_FLAG_INODE;
-
-	assert_spin_locked(&mark->lock);
-	assert_spin_locked(&group->mark_lock);
+	/* get ref for mark on list */
+	fsnotify_get_mark(mark);
 
 	spin_lock(&inode->i_lock);
+	spin_lock(&mark->lock);
 
-	mark->i.inode = inode;
+	mark->group = group;
+	mark->i.inode = igrab(inode);
+	mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
+	mark->flags |= FSNOTIFY_MARK_FLAG_INODE;
+	mark->flags |= FSNOTIFY_MARK_FLAG_OBJECT_PINNED;
 
 	/* is mark the first mark? */
 	if (hlist_empty(&inode->i_fsnotify_marks)) {
@@ -227,9 +230,20 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark,
 	/* mark should be the last entry.  last is the current last entry */
 	hlist_add_after_rcu(last, &mark->i.i_list);
 out:
+	spin_unlock(&mark->lock);
 	fsnotify_recalc_inode_mask_locked(inode);
 	spin_unlock(&inode->i_lock);
 
+	if (ret) {
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_OBJECT_PINNED;
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_INODE;
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
+		iput(mark->i.inode);
+		mark->group = NULL;
+		/* unget ref for mark on list */
+		fsnotify_put_mark(mark);
+	}
+
 	return ret;
 }
 
diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c
index 85eebff..35dc586 100644
--- a/fs/notify/vfsmount_mark.c
+++ b/fs/notify/vfsmount_mark.c
@@ -145,14 +145,17 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark,
 	struct hlist_node *node, *last = NULL;
 	int ret = 0;
 
-	mark->flags |= FSNOTIFY_MARK_FLAG_VFSMOUNT;
 
-	assert_spin_locked(&mark->lock);
-	assert_spin_locked(&group->mark_lock);
+	/* get ref for mark on list */
+	fsnotify_get_mark(mark);
 
 	spin_lock(&mnt->mnt_root->d_lock);
+	spin_lock(&mark->lock);
 
+	mark->group = group;
 	mark->m.mnt = mnt;
+	mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
+	mark->flags |= FSNOTIFY_MARK_FLAG_VFSMOUNT;
 
 	/* is mark the first mark? */
 	if (hlist_empty(&mnt->mnt_fsnotify_marks)) {
@@ -184,8 +187,18 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark,
 	/* mark should be the last entry.  last is the current last entry */
 	hlist_add_after_rcu(last, &mark->m.m_list);
 out:
+	spin_unlock(&mark->lock);
 	fsnotify_recalc_vfsmount_mask_locked(mnt);
 	spin_unlock(&mnt->mnt_root->d_lock);
 
+	if (ret) {
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_VFSMOUNT;
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
+		mark->m.mnt = NULL;
+		mark->group = NULL;
+		/* unget ref for mark on list */
+		fsnotify_put_mark(mark);
+	}
+
 	return ret;
 }
-- 
1.5.6.5

[PATCH 04/14] fsnotify: simplify fsnotify_destroy_mark()

[Lino Sanfilippo]

Reduce fsnotify_destroy_mark() to remove a mark from an inode or a vfsmount.
We dont put a mark on the destroy_list any more.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/mark.c |   68 +----------------------------------------------------
 1 files changed, 2 insertions(+), 66 deletions(-)

diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 247cbdf..916a291 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -110,74 +110,10 @@ static DECLARE_WAIT_QUEUE_HEAD(destroy_waitq);
  */
 void fsnotify_destroy_mark(struct fsnotify_mark *mark)
 {
-	struct fsnotify_group *group;
-	struct inode *inode = NULL;
-
-	spin_lock(&mark->lock);
-
-	group = mark->group;
-
-	/* something else already called this function on this mark */
-	if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
-		spin_unlock(&mark->lock);
-		return;
-	}
-
-	mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
-
-	/* 1 from caller and 1 for being on i_list/g_list */
-	BUG_ON(atomic_read(&mark->refcnt) < 2);
-
-	spin_lock(&group->mark_lock);
-
-	if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) {
-		inode = mark->i.inode;
-		fsnotify_destroy_inode_mark(mark);
-	} else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT)
+	if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT)
 		fsnotify_destroy_vfsmount_mark(mark);
 	else
-		BUG();
-
-	list_del_init(&mark->g_list);
-
-	spin_unlock(&group->mark_lock);
-	spin_unlock(&mark->lock);
-
-	spin_lock(&destroy_lock);
-	list_add(&mark->destroy_list, &destroy_list);
-	spin_unlock(&destroy_lock);
-	wake_up(&destroy_waitq);
-
-	/*
-	 * Some groups like to know that marks are being freed.  This is a
-	 * callback to the group function to let it know that this mark
-	 * is being freed.
-	 */
-	if (group->ops->freeing_mark)
-		group->ops->freeing_mark(mark, group);
-
-	/*
-	 * __fsnotify_update_child_dentry_flags(inode);
-	 *
-	 * I really want to call that, but we can't, we have no idea if the inode
-	 * still exists the second we drop the mark->lock.
-	 *
-	 * The next time an event arrive to this inode from one of it's children
-	 * __fsnotify_parent will see that the inode doesn't care about it's
-	 * children and will update all of these flags then.  So really this
-	 * is just a lazy update (and could be a perf win...)
-	 */
-
-	if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED))
-		iput(inode);
-
-	/*
-	 * it's possible that this group tried to destroy itself, but this
-	 * this mark was simultaneously being freed by inode.  If that's the
-	 * case, we finish freeing the group here.
-	 */
-	if (unlikely(atomic_dec_and_test(&group->num_marks)))
-		fsnotify_final_destroy_group(group);
+		fsnotify_destroy_inode_mark(mark);
 }
 
 void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask)
-- 
1.5.6.5

[PATCH 05/14] fsnotify: synchronize mark_srcu after a mark has been removed from a fsobject

[Lino Sanfilippo]

Instead of synchronizing mark_srcu after when a group is being destroyed,
we call it for each mark that is unlinked from its inode.
Synchronizing ensures that no more readers (callers of fsnotify()) are
referencing the mark when we decrement the marks ref count (and thus
possibly free it).
Since we can call synchronize_srcu without holding a lock, we dont need
to destroy the marks in a dedicated kernel thread any more.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/group.c         |    2 --
 fs/notify/inode_mark.c    |   38 +++++++++++++++++++++++++++++---------
 fs/notify/vfsmount_mark.c |   30 +++++++++++++++++++++++-------
 3 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/fs/notify/group.c b/fs/notify/group.c
index cc341d3..0dcf497 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -55,8 +55,6 @@ static void fsnotify_destroy_group(struct fsnotify_group *group)
 	/* clear all inode marks for this group */
 	fsnotify_clear_marks_by_group(group);
 
-	synchronize_srcu(&fsnotify_mark_srcu);
-
 	/* past the point of no return, matches the initial value of 1 */
 	if (atomic_dec_and_test(&group->num_marks))
 		fsnotify_final_destroy_group(group);
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 8d35a84..7403404 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -60,24 +60,44 @@ void fsnotify_recalc_inode_mask(struct inode *inode)
 
 void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
 {
-	struct inode *inode = mark->i.inode;
+	struct inode *inode;
 
-	assert_spin_locked(&mark->lock);
-	assert_spin_locked(&mark->group->mark_lock);
+	/* prepare removal from inode */
+	spin_lock(&mark->lock);
+	if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
+		spin_unlock(&mark->lock);
+		return;
+	}
+	inode = mark->i.inode;
+	mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
+	spin_unlock(&mark->lock);
 
+	/* remove mark from inode */
 	spin_lock(&inode->i_lock);
-
 	hlist_del_init_rcu(&mark->i.i_list);
-	mark->i.inode = NULL;
-
 	/*
 	 * this mark is now off the inode->i_fsnotify_marks list and we
 	 * hold the inode->i_lock, so this is the perfect time to update the
 	 * inode->i_fsnotify_mask
 	 */
 	fsnotify_recalc_inode_mask_locked(inode);
-
 	spin_unlock(&inode->i_lock);
+
+	/* wait until no readers (callers of fsnotify()) reference this
+	   mark any more */
+	synchronize_srcu(&fsnotify_mark_srcu);
+
+	/* do rest of removal */
+	spin_lock(&mark->lock);
+	if (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED) {
+		iput(inode);
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_OBJECT_PINNED;
+		mark->i.inode = NULL;
+	}
+	spin_unlock(&mark->lock);
+
+	/* release ref from list */
+	fsnotify_put_mark(mark);
 }
 
 /*
diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c
index 35dc586..20286e3 100644
--- a/fs/notify/vfsmount_mark.c
+++ b/fs/notify/vfsmount_mark.c
@@ -84,19 +84,35 @@ void fsnotify_recalc_vfsmount_mask(struct vfsmount *mnt)
 
 void fsnotify_destroy_vfsmount_mark(struct fsnotify_mark *mark)
 {
-	struct vfsmount *mnt = mark->m.mnt;
+	struct vfsmount *mnt;
 
-	assert_spin_locked(&mark->lock);
-	assert_spin_locked(&mark->group->mark_lock);
+	/* prepare removal from vfsmount */
+	spin_lock(&mark->lock);
+	if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
+		spin_unlock(&mark->lock);
+		return;
+	}
+	mnt = mark->m.mnt;
+	mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
+	spin_unlock(&mark->lock);
 
+	/* remove mark from vfsmount */
 	spin_lock(&mnt->mnt_root->d_lock);
-
 	hlist_del_init_rcu(&mark->m.m_list);
-	mark->m.mnt = NULL;
-
 	fsnotify_recalc_vfsmount_mask_locked(mnt);
-
 	spin_unlock(&mnt->mnt_root->d_lock);
+
+	/* wait until no readers (callers of fsnotify()) reference this
+	   mark any more */
+	synchronize_srcu(&fsnotify_mark_srcu);
+
+	/* do rest of removal */
+	spin_lock(&mark->lock);
+	mark->m.mnt = NULL;
+	spin_unlock(&mark->lock);
+
+	/* release ref from list */
+	fsnotify_put_mark(mark);
 }
 
 static struct fsnotify_mark *fsnotify_find_vfsmount_mark_locked(struct fsnotify_group *group,
-- 
1.5.6.5

[PATCH 06/14] fsnotify: clear fsobject marks with object lock held

[Lino Sanfilippo]

It is now possible to lock a mark with the fsobject lock held. So instead of
iterating the marks list of an inode/vfsmount and putting all marks on a temporary
free list and freeing that list afterwards, the marks are already freed
while the list is iterated.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/inode_mark.c           |   28 +++++++++++++------
 fs/notify/vfsmount_mark.c        |   55 ++++++++++++++++++++-----------------
 include/linux/fsnotify_backend.h |    2 -
 3 files changed, 49 insertions(+), 36 deletions(-)

diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 7403404..35bd4e3 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -105,22 +105,32 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
  */
 void fsnotify_clear_marks_by_inode(struct inode *inode)
 {
-	struct fsnotify_mark *mark, *lmark;
+	struct fsnotify_mark *mark;
 	struct hlist_node *pos, *n;
-	LIST_HEAD(free_list);
 
 	spin_lock(&inode->i_lock);
 	hlist_for_each_entry_safe(mark, pos, n, &inode->i_fsnotify_marks, i.i_list) {
-		list_add(&mark->i.free_i_list, &free_list);
-		hlist_del_init_rcu(&mark->i.i_list);
-		fsnotify_get_mark(mark);
-	}
-	spin_unlock(&inode->i_lock);
+		spin_lock(&mark->lock);
+		if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
+			spin_unlock(&mark->lock);
+			continue;
+		}
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
+		mark->i.inode = NULL;
 
-	list_for_each_entry_safe(mark, lmark, &free_list, i.free_i_list) {
-		fsnotify_destroy_mark(mark);
+		if (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED) {
+			iput(inode);
+			mark->flags &= ~FSNOTIFY_MARK_FLAG_OBJECT_PINNED;
+		}
+		spin_unlock(&mark->lock);
+
+		/* remove mark from inode */
+		hlist_del_init_rcu(&mark->i.i_list);
+		/* release ref from list */
 		fsnotify_put_mark(mark);
 	}
+	fsnotify_recalc_inode_mask_locked(inode);
+	spin_unlock(&inode->i_lock);
 }
 
 /*
diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c
index 20286e3..d253a39 100644
--- a/fs/notify/vfsmount_mark.c
+++ b/fs/notify/vfsmount_mark.c
@@ -30,31 +30,6 @@
 #include <linux/fsnotify_backend.h>
 #include "fsnotify.h"
 
-void fsnotify_clear_marks_by_mount(struct vfsmount *mnt)
-{
-	struct fsnotify_mark *mark, *lmark;
-	struct hlist_node *pos, *n;
-	LIST_HEAD(free_list);
-
-	spin_lock(&mnt->mnt_root->d_lock);
-	hlist_for_each_entry_safe(mark, pos, n, &mnt->mnt_fsnotify_marks, m.m_list) {
-		list_add(&mark->m.free_m_list, &free_list);
-		hlist_del_init_rcu(&mark->m.m_list);
-		fsnotify_get_mark(mark);
-	}
-	spin_unlock(&mnt->mnt_root->d_lock);
-
-	list_for_each_entry_safe(mark, lmark, &free_list, m.free_m_list) {
-		fsnotify_destroy_mark(mark);
-		fsnotify_put_mark(mark);
-	}
-}
-
-void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group)
-{
-	fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_MARK_FLAG_VFSMOUNT);
-}
-
 /*
  * Recalculate the mask of events relevant to a given vfsmount locked.
  */
@@ -71,6 +46,36 @@ static void fsnotify_recalc_vfsmount_mask_locked(struct vfsmount *mnt)
 	mnt->mnt_fsnotify_mask = new_mask;
 }
 
+void fsnotify_clear_marks_by_mount(struct vfsmount *mnt)
+{
+	struct fsnotify_mark *mark;
+	struct hlist_node *pos, *n;
+
+	spin_lock(&mnt->mnt_root->d_lock);
+	hlist_for_each_entry_safe(mark, pos, n, &mnt->mnt_fsnotify_marks, m.m_list) {
+		spin_lock(&mark->lock);
+		if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
+			spin_unlock(&mark->lock);
+			continue;
+		}
+		mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
+		mark->m.mnt = NULL;
+		spin_unlock(&mark->lock);
+
+		/* remove mark from vfsmount*/
+		hlist_del_init_rcu(&mark->m.m_list);
+		/* release ref from list */
+		fsnotify_put_mark(mark);
+	}
+	fsnotify_recalc_vfsmount_mask_locked(mnt);
+	spin_unlock(&mnt->mnt_root->d_lock);
+}
+
+void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group)
+{
+	fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_MARK_FLAG_VFSMOUNT);
+}
+
 /*
  * Recalculate the mnt->mnt_fsnotify_mask, or the mask of all FS_* event types
  * any notifier is interested in hearing for this mount point
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index b018e78..f6891f9 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -255,7 +255,6 @@ struct fsnotify_event {
 struct fsnotify_inode_mark {
 	struct inode *inode;		/* inode this mark is associated with */
 	struct hlist_node i_list;	/* list of marks by inode->i_fsnotify_marks */
-	struct list_head free_i_list;	/* tmp list used when freeing this mark */
 };
 
 /*
@@ -264,7 +263,6 @@ struct fsnotify_inode_mark {
 struct fsnotify_vfsmount_mark {
 	struct vfsmount *mnt;		/* vfsmount this mark is associated with */
 	struct hlist_node m_list;	/* list of marks by inode->i_fsnotify_marks */
-	struct list_head free_m_list;	/* tmp list used when freeing this mark */
 };
 
 /*
-- 
1.5.6.5

[PATCH 07/14] fsnotify: handle number of marks and group ref counting independently from each other

[Lino Sanfilippo]

With that patch ref counting of a group and its number of marks are handled
independently.
A group starts with a num_marks value of 0 instead of 1 and it does not depend
any longer on the number of marks wheather a group is finally destroyed.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/group.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/notify/group.c b/fs/notify/group.c
index 0dcf497..a407bac 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -54,10 +54,7 @@ static void fsnotify_destroy_group(struct fsnotify_group *group)
 {
 	/* clear all inode marks for this group */
 	fsnotify_clear_marks_by_group(group);
-
-	/* past the point of no return, matches the initial value of 1 */
-	if (atomic_dec_and_test(&group->num_marks))
-		fsnotify_final_destroy_group(group);
+	fsnotify_final_destroy_group(group);
 }
 
 /*
@@ -82,11 +79,7 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops)
 
 	/* set to 0 when there a no external references to this group */
 	atomic_set(&group->refcnt, 1);
-	/*
-	 * hits 0 when there are no external references AND no marks for
-	 * this group
-	 */
-	atomic_set(&group->num_marks, 1);
+	atomic_set(&group->num_marks, 0);
 
 	mutex_init(&group->mutex);
 	mutex_init(&group->notification_mutex);
-- 
1.5.6.5

[PATCH 08/14] fanotify: add an extra flag to mark_remove_from_mask that indicates wheather a mark could be destroyed

[Lino Sanfilippo]

This patch adds an extra flag to mark_remove_from_mask() to inform the caller if
all masks of a mark have been cleared and thus the mark could be destroyed.
With this we dont destroy the mark in the function itself any more but let the 
caller decide what to do on mark destruction.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/fanotify/fanotify_user.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 41bfdfd..a8d93e1 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -516,7 +516,8 @@ out:
 
 static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark,
 					    __u32 mask,
-					    unsigned int flags)
+					    unsigned int flags,
+					    int *destroy)
 {
 	__u32 oldmask;
 	int destroy_mark;
@@ -532,8 +533,7 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark,
 	destroy_mark = (!fsn_mark->mask && !fsn_mark->ignored_mask);
 	spin_unlock(&fsn_mark->lock);
 
-	if (destroy_mark)
-		fsnotify_destroy_mark(fsn_mark);
+	*destroy = destroy_mark;
 
 	return mask & oldmask;
 }
@@ -544,6 +544,7 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group,
 {
 	struct fsnotify_mark *fsn_mark = NULL;
 	__u32 removed;
+	int destroy_mark;
 	int ret;
 
 	mutex_lock(&group->mutex);
@@ -552,7 +553,11 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group,
 	if (!fsn_mark)
 		goto err;
 
-	removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags);
+	removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
+						 &destroy_mark);
+	if (destroy_mark)
+		fsnotify_destroy_mark(fsn_mark);
+
 	fsnotify_put_mark(fsn_mark);
 	if (removed & mnt->mnt_fsnotify_mask)
 		fsnotify_recalc_vfsmount_mask(mnt);
@@ -569,6 +574,7 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group,
 {
 	struct fsnotify_mark *fsn_mark = NULL;
 	__u32 removed;
+	int destroy_mark;
 	int ret;
 
 	mutex_lock(&group->mutex);
@@ -577,7 +583,11 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group,
 	if (!fsn_mark)
 		goto err;
 
-	removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags);
+	removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
+						 &destroy_mark);
+	if (destroy_mark)
+		fsnotify_destroy_mark(fsn_mark);
+
 	/* matches the fsnotify_find_inode_mark() */
 	fsnotify_put_mark(fsn_mark);
 	if (removed & inode->i_fsnotify_mask)
-- 
1.5.6.5

[PATCH 09/14] fsnotify: add new function fsnotify_remove_mark_locked()

[Lino Sanfilippo]

Adds the fucntion fsnotify_remove_mark_locked() which is the counterpart to
fsnotify_add_mark_locked():
The mark is removed from the mark list of its group, unlinked from the
fs object it is linked to, and the number of group marks is decreased.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/mark.c                 |   24 ++++++++++++++++++++++++
 include/linux/fsnotify_backend.h |    2 ++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 916a291..87d78b9 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -174,6 +174,30 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
 }
 
 /*
+ * Remove an attached mark from its group.
+ * If the mark is still linked to an fs object it will be unlinked, too.
+ * Requires groups mark_lock to be held.
+ */
+void fsnotify_remove_mark_locked(struct fsnotify_mark *mark)
+{
+	struct fsnotify_group *group = mark->group;
+
+	BUG_ON(!group);
+
+	list_del_init(&mark->g_list);
+	fsnotify_destroy_mark(mark);
+	/*
+	 * Some groups like to know that marks are being freed.
+	 * This is a callback to the group function to let it
+	 * know that this mark is being freed.
+	 */
+	if (group->ops->freeing_mark)
+		group->ops->freeing_mark(mark, group);
+	fsnotify_put_mark(mark);
+	atomic_dec(&group->num_marks);
+}
+
+/*
  * clear any marks in a group in which mark->flags & flags is true
  */
 void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group,
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index f6891f9..4c7f997 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -404,6 +404,8 @@ extern int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
 				    struct fsnotify_group *group,
 				    struct inode *inode, struct vfsmount *mnt,
 				    int allow_dups);
+/* remove mark from its group */
+extern void fsnotify_remove_mark_locked(struct fsnotify_mark *mark);
 /* given a mark, flag it to be freed when all references are dropped */
 extern void fsnotify_destroy_mark(struct fsnotify_mark *mark);
 /* run all the marks in a group, and clear all of the vfsmount marks */
-- 
1.5.6.5

[PATCH 10/14] fanotify,dnotify,inotify,audit: replace destroy_mark() with remove_mark_locked()

[Lino Sanfilippo]

fsnotify_destroy_mark() does not any longer remove a mark from its group. It
also does not any longer decrease the groups number of marks.
Thus all callers that used to call destroy_mark() should now call remove_mark_locked() 
with the group mutex held.
This patch replaces destroy_mark() with remove_mark_locked() and locks the
group mutex if needed.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 fs/notify/dnotify/dnotify.c          |    4 ++--
 fs/notify/fanotify/fanotify_user.c   |    4 ++--
 fs/notify/inotify/inotify_fsnotify.c |    7 +++++--
 fs/notify/inotify/inotify_user.c     |    4 +++-
 kernel/audit_tree.c                  |   28 +++++++++++++++++++++++-----
 kernel/audit_watch.c                 |   10 ++++++++--
 6 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index a81e97f..3dc1f25 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -200,7 +200,7 @@ void dnotify_flush(struct file *filp, fl_owner_t id)
 
 	/* nothing else could have found us thanks to the dnotify_group mutex */
 	if (dn_mark->dn == NULL)
-		fsnotify_destroy_mark(fsn_mark);
+		fsnotify_remove_mark_locked(fsn_mark);
 
 	mutex_unlock(&dnotify_group->mutex);
 
@@ -385,7 +385,7 @@ out:
 	spin_unlock(&fsn_mark->lock);
 
 	if (destroy)
-		fsnotify_destroy_mark(fsn_mark);
+		fsnotify_remove_mark_locked(fsn_mark);
 
 	mutex_unlock(&dnotify_group->mutex);
 	fsnotify_put_mark(fsn_mark);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index a8d93e1..afc2bb0 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -556,7 +556,7 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group,
 	removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
 						 &destroy_mark);
 	if (destroy_mark)
-		fsnotify_destroy_mark(fsn_mark);
+		fsnotify_remove_mark_locked(fsn_mark);
 
 	fsnotify_put_mark(fsn_mark);
 	if (removed & mnt->mnt_fsnotify_mask)
@@ -586,7 +586,7 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group,
 	removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
 						 &destroy_mark);
 	if (destroy_mark)
-		fsnotify_destroy_mark(fsn_mark);
+		fsnotify_remove_mark_locked(fsn_mark);
 
 	/* matches the fsnotify_find_inode_mark() */
 	fsnotify_put_mark(fsn_mark);
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index a91b69a..dec8b94 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -130,8 +130,11 @@ static int inotify_handle_event(struct fsnotify_group *group,
 			ret = PTR_ERR(added_event);
 	}
 
-	if (inode_mark->mask & IN_ONESHOT)
-		fsnotify_destroy_mark(inode_mark);
+	if (inode_mark->mask & IN_ONESHOT) {
+		mutex_lock(&group->mutex);
+		fsnotify_remove_mark_locked(&i_mark->fsn_mark);
+		mutex_unlock(&group->mutex);
+	}
 
 	return ret;
 }
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index bb91b53..bd31742 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -831,7 +831,9 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
 
 	ret = 0;
 
-	fsnotify_destroy_mark(&i_mark->fsn_mark);
+	mutex_lock(&group->mutex);
+	fsnotify_remove_mark_locked(&i_mark->fsn_mark);
+	mutex_unlock(&group->mutex);
 
 	/* match ref taken by inotify_idr_find */
 	fsnotify_put_mark(&i_mark->fsn_mark);
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 4b50ccc..1c3ce71 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -255,7 +255,11 @@ static void untag_chunk(struct node *p)
 		list_del_rcu(&chunk->hash);
 		spin_unlock(&hash_lock);
 		spin_unlock(&entry->lock);
-		fsnotify_destroy_mark(entry);
+
+		mutex_lock(&audit_tree_group->mutex);
+		fsnotify_remove_mark_locked(entry);
+		mutex_unlock(&audit_tree_group->mutex);
+
 		fsnotify_put_mark(entry);
 		goto out;
 	}
@@ -299,7 +303,11 @@ static void untag_chunk(struct node *p)
 		owner->root = new;
 	spin_unlock(&hash_lock);
 	spin_unlock(&entry->lock);
-	fsnotify_destroy_mark(entry);
+
+	mutex_lock(&audit_tree_group->mutex);
+	fsnotify_remove_mark_locked(entry);
+	mutex_unlock(&audit_tree_group->mutex);
+
 	fsnotify_put_mark(entry);
 	goto out;
 
@@ -339,7 +347,11 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree)
 		spin_unlock(&hash_lock);
 		chunk->dead = 1;
 		spin_unlock(&entry->lock);
-		fsnotify_destroy_mark(entry);
+
+		mutex_lock(&audit_tree_group->mutex);
+		fsnotify_remove_mark_locked(entry);
+		mutex_unlock(&audit_tree_group->mutex);
+
 		fsnotify_put_mark(entry);
 		return 0;
 	}
@@ -420,7 +432,9 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
 		spin_unlock(&chunk_entry->lock);
 		spin_unlock(&old_entry->lock);
 
-		fsnotify_destroy_mark(chunk_entry);
+		mutex_lock(&audit_tree_group->mutex);
+		fsnotify_remove_mark_locked(chunk_entry);
+		mutex_unlock(&audit_tree_group->mutex);
 
 		fsnotify_put_mark(chunk_entry);
 		fsnotify_put_mark(old_entry);
@@ -451,7 +465,11 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
 	spin_unlock(&hash_lock);
 	spin_unlock(&chunk_entry->lock);
 	spin_unlock(&old_entry->lock);
-	fsnotify_destroy_mark(old_entry);
+
+	mutex_lock(&audit_tree_group->mutex);
+	fsnotify_remove_mark_locked(old_entry);
+	mutex_unlock(&audit_tree_group->mutex);
+
 	fsnotify_put_mark(old_entry); /* pair to fsnotify_find mark_entry */
 	fsnotify_put_mark(old_entry); /* and kill it */
 	return 0;
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 6a57231..d859ec0 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -350,7 +350,9 @@ static void audit_remove_parent_watches(struct audit_parent *parent)
 	}
 	mutex_unlock(&audit_filter_mutex);
 
-	fsnotify_destroy_mark(&parent->mark);
+	mutex_lock(&audit_watch_group->mutex);
+	fsnotify_remove_mark_locked(&parent->mark);
+	mutex_unlock(&audit_watch_group->mutex);
 }
 
 /* Get path information necessary for adding watches. */
@@ -497,7 +499,11 @@ void audit_remove_watch_rule(struct audit_krule *krule)
 
 		if (list_empty(&parent->watches)) {
 			audit_get_parent(parent);
-			fsnotify_destroy_mark(&parent->mark);
+
+			mutex_lock(&audit_watch_group->mutex);
+			fsnotify_remove_mark_locked(&parent->mark);
+			mutex_unlock(&audit_watch_group->mutex);
+
 			audit_put_parent(parent);
 		}
 	}
-- 
1.5.6.5

Re: [PATCH 0/14] fsnotify: simplify locking

[Eric Paris]

On Wed, 2011-01-19 at 17:42 +0100, Lino Sanfilippo wrote:

> The main goal of these patches is to change the locking order to 
> 
> group->mark_lock
> inode->i_lock
> mark->lock

There was a LOT of thought that went into the object locking and
lifetime to make sure it was safe, but I agree it is, ummm, complex.
I'll look at these patches but offhand I seem to recall that (by
definition) inode->i_lock was always supposed to be the smallest lock
ever held.  I added Al a VFS guy, who might veto these patches just on
that alone......

-Eric