This patch series eliminates the need for the fsnotify group mutex which is
currently needed to ensure race free addition and removal to groups mark lists.
For this reason the locking order required to add or remove marks is turned from
1. mark lock
2. group mark_lock
3. inode/vfsmount lock
to
1. group mark_lock
2. mark lock
3. inode/vfsmount lock
Furthermore the group mark lock is turned from a spin lock to a mutex.
The patches apply against commit ef9bf3b7144bee6ce1da5616015cabc8771206af of
branch 'origin/for-next' from git.infradead.org/users/eparis/notify.git
[PATCH 1/7] fsnotify: take groups mark_lock before mark lock
Change locking order (see above).
[PATCH 2/7] fanotify: add an extra flag to mark_remove_from_mask that indicates wheather a mark could be destroyed
Add an extra flag to mark_remove_from_mask that indicates whether a mark could
be destroyed. This is in preparation for the next patches.
[PATCH 3/7] fsnotify: introduce fsnotify_remove_mark()
Introduce a new function fsnotify_remove_mark() which is a counterpart to
fsnotify_add_mark().
[PATCH 4/7] fsnotify: replace most calls of fsnotify_destroy_mark() with fsnotify_remove_mark()
Replace calls to fsnotify_destroy_mark() with calls to fsnotify_remove_mark().
This concerns all calls to destroy_mark() with exception of the ones called from
a disappearing fsobject.
[PATCH 5/7] fsnotify: use a mutex instead of a spinlock to protect a groups mark list
Turn the mark_list spinlock into a mutex.
[PATCH 6/7] fsnotify: introduce locked versions of fsnotify_add_mark() and fsnotify_remove_mark()
Introduced _locked versions of fsnotify_[add|remove]_mark that assume that the
mark list mutex is already held. This enables the caller to do custom tasks
while the mark list is protected.
[PATCH 7/7] fsnotify: replace the groups mutex with the groups mark list mutex
Replace the group mutex with the mark_list mutex and call the _locked() version of
fsnotify_[add|remove]_mark() while the mutex is held.
This patch replaces all calls of fsnotify_destroy_mark with calls to
fsnotify_remove_mark(). The exception is for fsnotify_clear_marks_by[inode|mount]()
where we dont have a group argumente in the caller available.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
fs/notify/dnotify/dnotify.c | 4 ++--
fs/notify/fanotify/fanotify_user.c | 4 ++--
fs/notify/inotify/inotify_fsnotify.c | 2 +-
fs/notify/inotify/inotify_user.c | 2 +-
kernel/audit_tree.c | 10 +++++-----
kernel/audit_watch.c | 4 ++--
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 89ec7e0..5dfd35a 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(fsn_mark, dnotify_group);
mutex_unlock(&dnotify_group->mutex);
@@ -384,7 +384,7 @@ out:
spin_unlock(&fsn_mark->lock);
if (destroy)
- fsnotify_destroy_mark(fsn_mark);
+ fsnotify_remove_mark(fsn_mark, dnotify_group);
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 848648f..c441b91 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(fsn_mark, group);
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(fsn_mark, group);
/* 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..c999fd3 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -131,7 +131,7 @@ static int inotify_handle_event(struct fsnotify_group *group,
}
if (inode_mark->mask & IN_ONESHOT)
- fsnotify_destroy_mark(inode_mark);
+ fsnotify_remove_mark(inode_mark, group);
return ret;
}
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 4cd5d5d..1637820 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -830,7 +830,7 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
ret = 0;
- fsnotify_destroy_mark(&i_mark->fsn_mark);
+ fsnotify_remove_mark(&i_mark->fsn_mark, group);
/* 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 37b2bea..07078ba 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -255,7 +255,7 @@ static void untag_chunk(struct node *p)
list_del_rcu(&chunk->hash);
spin_unlock(&hash_lock);
spin_unlock(&entry->lock);
- fsnotify_destroy_mark(entry);
+ fsnotify_remove_mark(entry, audit_tree_group);
fsnotify_put_mark(entry);
goto out;
}
@@ -298,7 +298,7 @@ static void untag_chunk(struct node *p)
owner->root = new;
spin_unlock(&hash_lock);
spin_unlock(&entry->lock);
- fsnotify_destroy_mark(entry);
+ fsnotify_remove_mark(entry, audit_tree_group);
fsnotify_put_mark(entry);
goto out;
@@ -338,7 +338,7 @@ 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);
+ fsnotify_remove_mark(entry, audit_tree_group);
fsnotify_put_mark(entry);
return 0;
}
@@ -418,7 +418,7 @@ 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);
+ fsnotify_remove_mark(chunk_entry, audit_tree_group);
fsnotify_put_mark(chunk_entry);
fsnotify_put_mark(old_entry);
@@ -449,7 +449,7 @@ 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);
+ fsnotify_remove_mark(old_entry, audit_tree_group);
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 d2e3c78..b50c857 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -349,7 +349,7 @@ static void audit_remove_parent_watches(struct audit_parent *parent)
}
mutex_unlock(&audit_filter_mutex);
- fsnotify_destroy_mark(&parent->mark);
+ fsnotify_remove_mark(&parent->mark, audit_watch_group);
}
/* Get path information necessary for adding watches. */
@@ -496,7 +496,7 @@ void audit_remove_watch_rule(struct audit_krule *krule)
if (list_empty(&parent->watches)) {
audit_get_parent(parent);
- fsnotify_destroy_mark(&parent->mark);
+ fsnotify_remove_mark(&parent->mark, audit_watch_group);
audit_put_parent(parent);
}
}
--
1.5.6.5
Replaces the groups mark_lock spinlock with a mutex. Using a mutex instead
of a spinlock results in more flexibility (i.e it allows to sleep while the
lock is held).
Signed-off-by: Lino Sanfilippo <[email protected]>
---
fs/notify/group.c | 2 +-
fs/notify/inode_mark.c | 2 --
fs/notify/mark.c | 20 ++++++++++----------
fs/notify/vfsmount_mark.c | 2 --
include/linux/fsnotify_backend.h | 2 +-
5 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/fs/notify/group.c b/fs/notify/group.c
index cc341d3..a45bbfb 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -96,7 +96,7 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops)
init_waitqueue_head(&group->notification_waitq);
group->max_events = UINT_MAX;
- spin_lock_init(&group->mark_lock);
+ mutex_init(&group->mark_mutex);
INIT_LIST_HEAD(&group->marks_list);
group->ops = ops;
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 4c29fcf..d438f11 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -63,7 +63,6 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
struct inode *inode = mark->i.inode;
assert_spin_locked(&mark->lock);
- assert_spin_locked(&mark->group->mark_lock);
spin_lock(&inode->i_lock);
@@ -191,7 +190,6 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark,
mark->flags |= FSNOTIFY_MARK_FLAG_INODE;
assert_spin_locked(&mark->lock);
- assert_spin_locked(&group->mark_lock);
spin_lock(&inode->i_lock);
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 3b00203..30b72b2 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -123,7 +123,7 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
group = mark->group;
spin_unlock(&mark->lock);
- spin_lock(&group->mark_lock);
+ mutex_lock(&group->mark_mutex);
spin_lock(&mark->lock);
/* 1 from caller and 1 for being on i_list/g_list */
@@ -140,7 +140,7 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
list_del_init(&mark->g_list);
- spin_unlock(&group->mark_lock);
+ mutex_unlock(&group->mark_mutex);
spin_lock(&destroy_lock);
list_add(&mark->destroy_list, &destroy_list);
@@ -216,7 +216,7 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
* group->mark_lock
* inode->i_lock
*/
- spin_lock(&group->mark_lock);
+ mutex_lock(&group->mark_mutex);
spin_lock(&mark->lock);
mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
@@ -238,7 +238,7 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
fsnotify_set_mark_mask_locked(mark, mark->mask);
spin_unlock(&mark->lock);
- spin_unlock(&group->mark_lock);
+ mutex_unlock(&group->mark_mutex);
if (inode)
__fsnotify_update_child_dentry_flags(inode);
@@ -251,7 +251,7 @@ err:
atomic_dec(&group->num_marks);
spin_unlock(&mark->lock);
- spin_unlock(&group->mark_lock);
+ mutex_unlock(&group->mark_mutex);
spin_lock(&destroy_lock);
list_add(&mark->destroy_list, &destroy_list);
@@ -270,13 +270,13 @@ void fsnotify_remove_mark(struct fsnotify_mark *mark,
{
struct inode *inode = NULL;
- spin_lock(&group->mark_lock);
+ mutex_lock(&group->mark_mutex);
spin_lock(&mark->lock);
/* something else already called this function on this mark */
if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
spin_unlock(&mark->lock);
- spin_unlock(&group->mark_lock);
+ mutex_unlock(&group->mark_mutex);
return;
}
@@ -296,7 +296,7 @@ void fsnotify_remove_mark(struct fsnotify_mark *mark,
spin_unlock(&mark->lock);
list_del_init(&mark->g_list);
- spin_unlock(&group->mark_lock);
+ mutex_unlock(&group->mark_mutex);
spin_lock(&destroy_lock);
list_add(&mark->destroy_list, &destroy_list);
@@ -344,7 +344,7 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group,
struct fsnotify_mark *lmark, *mark;
LIST_HEAD(free_list);
- spin_lock(&group->mark_lock);
+ mutex_lock(&group->mark_mutex);
list_for_each_entry_safe(mark, lmark, &group->marks_list, g_list) {
if (mark->flags & flags) {
list_add(&mark->free_g_list, &free_list);
@@ -352,7 +352,7 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group,
fsnotify_get_mark(mark);
}
}
- spin_unlock(&group->mark_lock);
+ mutex_unlock(&group->mark_mutex);
list_for_each_entry_safe(mark, lmark, &free_list, free_g_list) {
fsnotify_destroy_mark(mark);
diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c
index 85eebff..04f5929 100644
--- a/fs/notify/vfsmount_mark.c
+++ b/fs/notify/vfsmount_mark.c
@@ -87,7 +87,6 @@ void fsnotify_destroy_vfsmount_mark(struct fsnotify_mark *mark)
struct vfsmount *mnt = mark->m.mnt;
assert_spin_locked(&mark->lock);
- assert_spin_locked(&mark->group->mark_lock);
spin_lock(&mnt->mnt_root->d_lock);
@@ -148,7 +147,6 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark,
mark->flags |= FSNOTIFY_MARK_FLAG_VFSMOUNT;
assert_spin_locked(&mark->lock);
- assert_spin_locked(&group->mark_lock);
spin_lock(&mnt->mnt_root->d_lock);
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index c7bfee6..29ed5f8 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -142,7 +142,7 @@ struct fsnotify_group {
unsigned int priority;
/* stores all fastpath marks assoc with this group so they can be cleaned on unregister */
- spinlock_t mark_lock; /* protect marks_list */
+ struct mutex mark_mutex; /* protect marks_list */
atomic_t num_marks; /* 1 for each mark and 1 for not being
* past the point of no return when freeing
* a group */
--
1.5.6.5
This patch introduces fsnotify_add_mark_locked() and fsnotify_remove_mark_locked()
which are essentially the same as fsnotify_add_mark and fsnotify_remove_mark() but
assume that the caller has already taken the groups mark mutex.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
fs/notify/mark.c | 69 +++++++++++++++++++++++---------------
include/linux/fsnotify_backend.h | 6 +++
2 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 30b72b2..11cfedc 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -196,27 +196,12 @@ void fsnotify_set_mark_ignored_mask_locked(struct fsnotify_mark *mark, __u32 mas
mark->ignored_mask = mask;
}
-/*
- * 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.
- */
-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;
- BUG_ON(inode && mnt);
- BUG_ON(!inode && !mnt);
-
- /*
- * LOCKING ORDER!!!!
- * mark->lock
- * group->mark_lock
- * inode->i_lock
- */
- mutex_lock(&group->mark_mutex);
spin_lock(&mark->lock);
mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
@@ -238,7 +223,6 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
fsnotify_set_mark_mask_locked(mark, mark->mask);
spin_unlock(&mark->lock);
- mutex_unlock(&group->mark_mutex);
if (inode)
__fsnotify_update_child_dentry_flags(inode);
@@ -251,7 +235,6 @@ err:
atomic_dec(&group->num_marks);
spin_unlock(&mark->lock);
- mutex_unlock(&group->mark_mutex);
spin_lock(&destroy_lock);
list_add(&mark->destroy_list, &destroy_list);
@@ -262,21 +245,42 @@ err:
}
/*
- * Remove a mark from a given group and the fsobject.
- * Must not be called for a mark that is not on the groups mark list.
+ * 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.
*/
-void fsnotify_remove_mark(struct fsnotify_mark *mark,
- struct fsnotify_group *group)
+int fsnotify_add_mark(struct fsnotify_mark *mark,
+ struct fsnotify_group *group, struct inode *inode,
+ struct vfsmount *mnt, int allow_dups)
{
- struct inode *inode = NULL;
+ int ret;
+
+ BUG_ON(inode && mnt);
+ BUG_ON(!inode && !mnt);
+ /*
+ * LOCKING ORDER!!!!
+ * mark->lock
+ * group->mark_lock
+ * inode->i_lock
+ */
mutex_lock(&group->mark_mutex);
+ ret = fsnotify_add_mark_locked(mark, group, inode, mnt, allow_dups);
+ mutex_unlock(&group->mark_mutex);
+
+ return ret;
+}
+
+void fsnotify_remove_mark_locked(struct fsnotify_mark *mark,
+ struct fsnotify_group *group)
+{
+ struct inode *inode = NULL;
+
spin_lock(&mark->lock);
/* something else already called this function on this mark */
if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
spin_unlock(&mark->lock);
- mutex_unlock(&group->mark_mutex);
return;
}
@@ -296,7 +300,6 @@ void fsnotify_remove_mark(struct fsnotify_mark *mark,
spin_unlock(&mark->lock);
list_del_init(&mark->g_list);
- mutex_unlock(&group->mark_mutex);
spin_lock(&destroy_lock);
list_add(&mark->destroy_list, &destroy_list);
@@ -336,6 +339,18 @@ void fsnotify_remove_mark(struct fsnotify_mark *mark,
}
/*
+ * Remove a mark from a given group and the fsobject.
+ * Must not be called for a mark that is not on the groups mark list.
+ */
+void fsnotify_remove_mark(struct fsnotify_mark *mark,
+ struct fsnotify_group *group)
+{
+ mutex_lock(&group->mark_mutex);
+ fsnotify_remove_mark_locked(mark, group);
+ mutex_unlock(&group->mark_mutex);
+}
+
+/*
* 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 29ed5f8..073f6a9 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -404,9 +404,15 @@ 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);
/* remove a mark from a given group and fsobject */
extern void fsnotify_remove_mark(struct fsnotify_mark *mark,
struct fsnotify_group *group);
+extern void fsnotify_remove_mark_locked(struct fsnotify_mark *mark,
+ struct fsnotify_group *group);
/* 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
This patch replaces the group mutex with the group mark_mutex to ensure
race-free addition and removal of marks to/from groups.
It also replaces calls to fsnotify_[add|remove]_mark() with calls to
fsnotify_[add|remove]_mark_locked() whenever the group mark_mutex is already
held.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
fs/notify/dnotify/dnotify.c | 13 +++++++------
fs/notify/fanotify/fanotify_user.c | 24 ++++++++++++------------
fs/notify/group.c | 1 -
include/linux/fsnotify_backend.h | 1 -
4 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 5dfd35a..9aa4ccd 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -182,7 +182,7 @@ void dnotify_flush(struct file *filp, fl_owner_t id)
return;
dn_mark = container_of(fsn_mark, struct dnotify_mark, fsn_mark);
- mutex_lock(&dnotify_group->mutex);
+ mutex_lock(&dnotify_group->mark_mutex);
spin_lock(&fsn_mark->lock);
prev = &dn_mark->dn;
@@ -200,9 +200,9 @@ 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_remove_mark(fsn_mark, dnotify_group);
+ fsnotify_remove_mark_locked(fsn_mark, dnotify_group);
- mutex_unlock(&dnotify_group->mutex);
+ mutex_unlock(&dnotify_group->mark_mutex);
fsnotify_put_mark(fsn_mark);
}
@@ -325,7 +325,7 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
new_dn_mark->dn = NULL;
/* this is needed to prevent the fcntl/close race described below */
- mutex_lock(&dnotify_group->mutex);
+ mutex_lock(&dnotify_group->mark_mutex);
/* add the new_fsn_mark or find an old one. */
fsn_mark = fsnotify_find_inode_mark(dnotify_group, inode);
@@ -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;
@@ -386,7 +387,7 @@ out:
if (destroy)
fsnotify_remove_mark(fsn_mark, dnotify_group);
- mutex_unlock(&dnotify_group->mutex);
+ mutex_unlock(&dnotify_group->mark_mutex);
fsnotify_put_mark(fsn_mark);
out_err:
if (new_fsn_mark)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index c441b91..f52b47b 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -547,7 +547,7 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group,
int destroy_mark;
int ret;
- mutex_lock(&group->mutex);
+ mutex_lock(&group->mark_mutex);
ret = -ENOENT;
fsn_mark = fsnotify_find_vfsmount_mark(group, mnt);
if (!fsn_mark)
@@ -556,14 +556,14 @@ 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_remove_mark(fsn_mark, group);
+ fsnotify_remove_mark_locked(fsn_mark, group);
fsnotify_put_mark(fsn_mark);
if (removed & mnt->mnt_fsnotify_mask)
fsnotify_recalc_vfsmount_mask(mnt);
ret = 0;
err:
- mutex_unlock(&group->mutex);
+ mutex_unlock(&group->mark_mutex);
return ret;
}
@@ -577,7 +577,7 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group,
int destroy_mark;
int ret;
- mutex_lock(&group->mutex);
+ mutex_lock(&group->mark_mutex);
ret = -ENOENT;
fsn_mark = fsnotify_find_inode_mark(group, inode);
if (!fsn_mark)
@@ -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_remove_mark(fsn_mark, group);
+ fsnotify_remove_mark_locked(fsn_mark, group);
/* matches the fsnotify_find_inode_mark() */
fsnotify_put_mark(fsn_mark);
@@ -594,7 +594,7 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group,
fsnotify_recalc_inode_mask(inode);
ret = 0;
err:
- mutex_unlock(&group->mutex);
+ mutex_unlock(&group->mark_mutex);
return ret;
}
@@ -634,7 +634,7 @@ static int fanotify_add_vfsmount_mark(struct fsnotify_group *group,
__u32 added;
int ret;
- mutex_lock(&group->mutex);
+ mutex_lock(&group->mark_mutex);
fsn_mark = fsnotify_find_vfsmount_mark(group, mnt);
if (!fsn_mark) {
ret = -ENOSPC;
@@ -648,7 +648,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;
}
@@ -660,7 +660,7 @@ static int fanotify_add_vfsmount_mark(struct fsnotify_group *group,
err2:
fsnotify_put_mark(fsn_mark);
err:
- mutex_unlock(&group->mutex);
+ mutex_unlock(&group->mark_mutex);
return ret;
}
@@ -684,7 +684,7 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,
(atomic_read(&inode->i_writecount) > 0))
return 0;
- mutex_lock(&group->mutex);
+ mutex_lock(&group->mark_mutex);
fsn_mark = fsnotify_find_inode_mark(group, inode);
if (!fsn_mark) {
ret = -ENOSPC;
@@ -698,7 +698,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;
}
@@ -710,7 +710,7 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,
err2:
fsnotify_put_mark(fsn_mark);
err:
- mutex_unlock(&group->mutex);
+ mutex_unlock(&group->mark_mutex);
return ret;
}
diff --git a/fs/notify/group.c b/fs/notify/group.c
index a45bbfb..c86e3fb 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -90,7 +90,6 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops)
*/
atomic_set(&group->num_marks, 1);
- mutex_init(&group->mutex);
mutex_init(&group->notification_mutex);
INIT_LIST_HEAD(&group->notification_list);
init_waitqueue_head(&group->notification_waitq);
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 073f6a9..e4100db 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -125,7 +125,6 @@ struct fsnotify_group {
const struct fsnotify_ops *ops; /* how this group handles things */
- struct mutex mutex;
/* needed to send notification to userspace */
struct mutex notification_mutex; /* protect the notification_list */
struct list_head notification_list; /* list of event_holder this group needs to send to userspace */
--
1.5.6.5
This patch introduces fsnotify_remove_mark() which is the counterpart to
fsnotify_add_mark().
The implementation is equal to that of fsnotify_destroy_mark() with the difference
that the group is passed as a function argument and the group lock is taken first.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
fs/notify/mark.c | 74 ++++++++++++++++++++++++++++++++++++++
include/linux/fsnotify_backend.h | 3 ++
2 files changed, 77 insertions(+), 0 deletions(-)
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 1ac0447..3b00203 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -262,6 +262,80 @@ err:
}
/*
+ * Remove a mark from a given group and the fsobject.
+ * Must not be called for a mark that is not on the groups mark list.
+ */
+void fsnotify_remove_mark(struct fsnotify_mark *mark,
+ struct fsnotify_group *group)
+{
+ struct inode *inode = NULL;
+
+ spin_lock(&group->mark_lock);
+ spin_lock(&mark->lock);
+
+ /* something else already called this function on this mark */
+ if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
+ spin_unlock(&mark->lock);
+ spin_unlock(&group->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);
+
+
+ if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) {
+ inode = mark->i.inode;
+ fsnotify_destroy_inode_mark(mark);
+ } else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT)
+ fsnotify_destroy_vfsmount_mark(mark);
+ else
+ BUG();
+ spin_unlock(&mark->lock);
+
+ list_del_init(&mark->g_list);
+ spin_unlock(&group->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);
+}
+
+/*
* 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 6a3c660..c7bfee6 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -404,6 +404,9 @@ 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);
+/* remove a mark from a given group and fsobject */
+extern void fsnotify_remove_mark(struct fsnotify_mark *mark,
+ struct fsnotify_group *group);
/* 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
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 the mark could be destroyed.
With this we dont destroy the mark implicitly in the function itself any more
but let the caller handle it.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
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 2d4925b..848648f 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
Race-free addition and removal of a mark to a groups mark list would be easier
if we could lock the mark list of group before we lock the specific mark.
This patch changes the order used to add/remove marks to/from mark lists from
1. mark->lock
2. group->mark_lock
3. inode->i_lock
to
1. group->mark_lock
2. mark->lock
3. inode->i_lock
Signed-off-by: Lino Sanfilippo <[email protected]>
---
fs/notify/mark.c | 21 +++++++++------------
1 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 28b64eb..1ac0447 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -114,22 +114,21 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
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;
+ group = mark->group;
+ spin_unlock(&mark->lock);
+
+ spin_lock(&group->mark_lock);
+ spin_lock(&mark->lock);
/* 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);
@@ -137,11 +136,11 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
fsnotify_destroy_vfsmount_mark(mark);
else
BUG();
+ spin_unlock(&mark->lock);
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);
@@ -217,8 +216,8 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
* group->mark_lock
* inode->i_lock
*/
- spin_lock(&mark->lock);
spin_lock(&group->mark_lock);
+ spin_lock(&mark->lock);
mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
mark->group = group;
@@ -235,13 +234,11 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
BUG();
if (ret)
goto err;
-
- spin_unlock(&group->mark_lock);
-
/* this will pin the object if appropriate */
fsnotify_set_mark_mask_locked(mark, mark->mask);
spin_unlock(&mark->lock);
+ spin_unlock(&group->mark_lock);
if (inode)
__fsnotify_update_child_dentry_flags(inode);
@@ -253,8 +250,8 @@ err:
mark->group = NULL;
atomic_dec(&group->num_marks);
- spin_unlock(&group->mark_lock);
spin_unlock(&mark->lock);
+ spin_unlock(&group->mark_lock);
spin_lock(&destroy_lock);
list_add(&mark->destroy_list, &destroy_list);
--
1.5.6.5