Control: tag -1 patch Here's an untested (except that it compiles) backport of the fixes from Linux 3.8 to wheezy. Would you mind testing these? Instructions for building a patched kernel are at <http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official>. Ben. -- Ben Hutchings Sturgeon's Law: Ninety percent of everything is crap.
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:45 +0200
Subject: [01/10] inotify, fanotify: replace fsnotify_put_group() with
fsnotify_destroy_group()
commit d8153d4d8b7b6141770e1416c4a338161205ed1b upstream.
Currently in fsnotify_put_group() the ref count of a group is decremented and if
it becomes 0 fsnotify_destroy_group() is called. Since a groups ref count is only
at group creation set to 1 and never increased after that a call to fsnotify_put_group()
always results in a call to fsnotify_destroy_group().
With this patch fsnotify_destroy_group() is called directly.
Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/fanotify/fanotify_user.c | 14 +++++++-------
fs/notify/group.c | 2 +-
fs/notify/inotify/inotify_user.c | 8 +++-----
include/linux/fsnotify_backend.h | 3 ++-
4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index d438036..82ae6d7 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -415,7 +415,7 @@ static int fanotify_release(struct inode *ignored, struct file *file)
wake_up(&group->fanotify_data.access_waitq);
#endif
/* matches the fanotify_init->fsnotify_alloc_group */
- fsnotify_put_group(group);
+ fsnotify_destroy_group(group);
return 0;
}
@@ -728,13 +728,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
break;
default:
fd = -EINVAL;
- goto out_put_group;
+ goto out_destroy_group;
}
if (flags & FAN_UNLIMITED_QUEUE) {
fd = -EPERM;
if (!capable(CAP_SYS_ADMIN))
- goto out_put_group;
+ goto out_destroy_group;
group->max_events = UINT_MAX;
} else {
group->max_events = FANOTIFY_DEFAULT_MAX_EVENTS;
@@ -743,7 +743,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
if (flags & FAN_UNLIMITED_MARKS) {
fd = -EPERM;
if (!capable(CAP_SYS_ADMIN))
- goto out_put_group;
+ goto out_destroy_group;
group->fanotify_data.max_marks = UINT_MAX;
} else {
group->fanotify_data.max_marks = FANOTIFY_DEFAULT_MAX_MARKS;
@@ -751,12 +751,12 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
fd = anon_inode_getfd("[fanotify]", &fanotify_fops, group, f_flags);
if (fd < 0)
- goto out_put_group;
+ goto out_destroy_group;
return fd;
-out_put_group:
- fsnotify_put_group(group);
+out_destroy_group:
+ fsnotify_destroy_group(group);
return fd;
}
diff --git a/fs/notify/group.c b/fs/notify/group.c
index 63fc294..cfda328 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -50,7 +50,7 @@ void fsnotify_final_destroy_group(struct fsnotify_group *group)
* situtation, the fsnotify_final_destroy_group will get called when that final
* mark is freed.
*/
-static void fsnotify_destroy_group(struct fsnotify_group *group)
+void fsnotify_destroy_group(struct fsnotify_group *group)
{
/* clear all inode marks for this group */
fsnotify_clear_marks_by_group(group);
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 8445fbc..dbafbfc 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -293,10 +293,8 @@ static int inotify_release(struct inode *ignored, struct file *file)
pr_debug("%s: group=%p\n", __func__, group);
- fsnotify_clear_marks_by_group(group);
-
/* free this group, matching get was inotify_init->fsnotify_obtain_group */
- fsnotify_put_group(group);
+ fsnotify_destroy_group(group);
return 0;
}
@@ -712,7 +710,7 @@ static struct fsnotify_group *inotify_new_group(unsigned int max_events)
if (atomic_inc_return(&group->inotify_data.user->inotify_devs) >
inotify_max_user_instances) {
- fsnotify_put_group(group);
+ fsnotify_destroy_group(group);
return ERR_PTR(-EMFILE);
}
@@ -741,7 +739,7 @@ SYSCALL_DEFINE1(inotify_init1, int, flags)
ret = anon_inode_getfd("inotify", &inotify_fops, group,
O_RDONLY | flags);
if (ret < 0)
- fsnotify_put_group(group);
+ fsnotify_destroy_group(group);
return ret;
}
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 63d966d..d2ad345 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -364,7 +364,8 @@ static inline void __fsnotify_d_instantiate(struct dentry *dentry, struct inode
extern struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops);
/* drop reference on a group from fsnotify_alloc_group */
extern void fsnotify_put_group(struct fsnotify_group *group);
-
+/* destroy group */
+extern void fsnotify_destroy_group(struct fsnotify_group *group);
/* take a reference to an event */
extern void fsnotify_get_event(struct fsnotify_event *event);
extern void fsnotify_put_event(struct fsnotify_event *event);
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:46 +0200
Subject: [02/10] fsnotify: introduce fsnotify_get_group()
commit 986129520479d689962a42c31acdeaf854ac91f5 upstream.
Introduce fsnotify_get_group() which increments the reference counter of a group.
Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/group.c | 8 ++++++++
include/linux/fsnotify_backend.h | 4 +++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/notify/group.c b/fs/notify/group.c
index cfda328..1d57c35 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -63,6 +63,14 @@ void fsnotify_destroy_group(struct fsnotify_group *group)
}
/*
+ * Get reference to a group.
+ */
+void fsnotify_get_group(struct fsnotify_group *group)
+{
+ atomic_inc(&group->refcnt);
+}
+
+/*
* Drop a reference to a group. Free it if it's through.
*/
void fsnotify_put_group(struct fsnotify_group *group)
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index d2ad345..e76cef7 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -360,8 +360,10 @@ static inline void __fsnotify_d_instantiate(struct dentry *dentry, struct inode
/* called from fsnotify listeners, such as fanotify or dnotify */
-/* get a reference to an existing or create a new group */
+/* create a new group */
extern struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops);
+/* get reference to a group */
+extern void fsnotify_get_group(struct fsnotify_group *group);
/* drop reference on a group from fsnotify_alloc_group */
extern void fsnotify_put_group(struct fsnotify_group *group);
/* destroy group */
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:47 +0200
Subject: [03/10] fsnotify: use reference counting for groups
commit 23e964c284ca0a767b80a30482bd53b059d30391 upstream.
Get a group ref for each mark that is added to the groups list and release that
ref when the mark is freed in fsnotify_put_mark().
We also use get a group reference for duplicated marks and for private event
data.
Now we dont free a group any more when the number of marks becomes 0 but when
the groups ref count does. Since this will only happen when all marks are removed
from a groups mark list, we dont have to set the groups number of marks to 1 at
group creation.
Beside clearing all marks in fsnotify_destroy_group() we do also flush the
groups event queue. This is since events may hold references to groups (due to
private event data) and we have to put those references first before we get a
chance to put the final ref, which will result in a call to
fsnotify_final_destroy_group().
Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/group.c | 28 ++++++++++------------------
fs/notify/inotify/inotify_fsnotify.c | 2 ++
fs/notify/inotify/inotify_user.c | 1 +
fs/notify/mark.c | 24 ++++++++++++++----------
4 files changed, 27 insertions(+), 28 deletions(-)
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -34,9 +34,6 @@
*/
void fsnotify_final_destroy_group(struct fsnotify_group *group)
{
- /* clear the notification queue of all events */
- fsnotify_flush_notify(group);
-
if (group->ops->free_group_priv)
group->ops->free_group_priv(group);
@@ -44,12 +41,10 @@ void fsnotify_final_destroy_group(struct
}
/*
- * Trying to get rid of a group. We need to first get rid of any outstanding
- * allocations and then free the group. Remember that fsnotify_clear_marks_by_group
- * could miss marks that are being freed by inode and those marks could still
- * hold a reference to this group (via group->num_marks) If we get into that
- * situtation, the fsnotify_final_destroy_group will get called when that final
- * mark is freed.
+ * Trying to get rid of a group. Remove all marks, flush all events and release
+ * the group reference.
+ * Note that another thread calling fsnotify_clear_marks_by_group() may still
+ * hold a ref to the group.
*/
void fsnotify_destroy_group(struct fsnotify_group *group)
{
@@ -58,9 +53,10 @@ void fsnotify_destroy_group(struct fsnot
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);
+ /* clear the notification queue of all events */
+ fsnotify_flush_notify(group);
+
+ fsnotify_put_group(group);
}
/*
@@ -77,7 +73,7 @@ void fsnotify_get_group(struct fsnotify_
void fsnotify_put_group(struct fsnotify_group *group)
{
if (atomic_dec_and_test(&group->refcnt))
- fsnotify_destroy_group(group);
+ fsnotify_final_destroy_group(group);
}
EXPORT_SYMBOL_GPL(fsnotify_put_group);
@@ -94,11 +90,7 @@ struct fsnotify_group *fsnotify_alloc_gr
/* 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->notification_mutex);
INIT_LIST_HEAD(&group->notification_list);
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -118,6 +118,7 @@ static int inotify_handle_event(struct f
fsn_event_priv = &event_priv->fsnotify_event_priv_data;
+ fsnotify_get_group(group);
fsn_event_priv->group = group;
event_priv->wd = wd;
@@ -210,6 +211,7 @@ void inotify_free_event_priv(struct fsno
event_priv = container_of(fsn_event_priv, struct inotify_event_private_data,
fsnotify_event_priv_data);
+ fsnotify_put_group(fsn_event_priv->group);
kmem_cache_free(event_priv_cachep, event_priv);
}
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -531,6 +531,7 @@ void inotify_ignored_and_remove_idr(stru
fsn_event_priv = &event_priv->fsnotify_event_priv_data;
+ fsnotify_get_group(group);
fsn_event_priv->group = group;
event_priv->wd = i_mark->wd;
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -109,8 +109,11 @@ void fsnotify_get_mark(struct fsnotify_m
void fsnotify_put_mark(struct fsnotify_mark *mark)
{
- if (atomic_dec_and_test(&mark->refcnt))
+ if (atomic_dec_and_test(&mark->refcnt)) {
+ if (mark->group)
+ fsnotify_put_group(mark->group);
mark->free_mark(mark);
+ }
}
EXPORT_SYMBOL_GPL(fsnotify_put_mark);
@@ -126,12 +129,13 @@ void fsnotify_destroy_mark(struct fsnoti
spin_lock(&mark->lock);
+ fsnotify_get_group(mark->group);
group = mark->group;
/* something else already called this function on this mark */
if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
spin_unlock(&mark->lock);
- return;
+ goto put_group;
}
mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
@@ -178,19 +182,15 @@ void fsnotify_destroy_mark(struct fsnoti
if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED))
iput(inode);
-
/*
* We don't necessarily have a ref on mark from caller so the above iput
* may have already destroyed it. Don't touch from now on.
*/
- /*
- * 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);
+ atomic_dec(&group->num_marks);
+
+put_group:
+ fsnotify_put_group(group);
}
EXPORT_SYMBOL_GPL(fsnotify_destroy_mark);
@@ -236,6 +236,7 @@ int fsnotify_add_mark(struct fsnotify_ma
mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
+ fsnotify_get_group(group);
mark->group = group;
list_add(&mark->g_list, &group->marks_list);
atomic_inc(&group->num_marks);
@@ -267,6 +268,7 @@ int fsnotify_add_mark(struct fsnotify_ma
err:
mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
list_del_init(&mark->g_list);
+ fsnotify_put_group(group);
mark->group = NULL;
atomic_dec(&group->num_marks);
@@ -320,6 +322,8 @@ void fsnotify_duplicate_mark(struct fsno
assert_spin_locked(&old->lock);
new->i.inode = old->i.inode;
new->m.mnt = old->m.mnt;
+ if (old->group)
+ fsnotify_get_group(old->group);
new->group = old->group;
new->mask = old->mask;
new->free_mark = old->free_mark;
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:48 +0200
Subject: [04/10] fsnotify: take groups mark_lock before mark lock
commit 104d06f08ea59247cb0e7e548c5a5d22d21dcfd5 upstream.
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 <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/mark.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 3c7a169..32447dc 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -127,20 +127,27 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
struct inode *inode = NULL;
spin_lock(&mark->lock);
-
+ /* dont get the group from a mark that is not alive yet */
+ if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
+ spin_unlock(&mark->lock);
+ return;
+ }
fsnotify_get_group(mark->group);
group = mark->group;
+ spin_unlock(&mark->lock);
+
+ 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);
goto put_group;
}
mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
- spin_lock(&group->mark_lock);
-
if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) {
inode = mark->i.inode;
fsnotify_destroy_inode_mark(mark);
@@ -151,8 +158,8 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
list_del_init(&mark->g_list);
- 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);
@@ -225,13 +232,13 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
/*
* LOCKING ORDER!!!!
- * mark->lock
* group->mark_lock
+ * mark->lock
* inode->i_lock
*/
- spin_lock(&mark->lock);
spin_lock(&group->mark_lock);
+ spin_lock(&mark->lock);
mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
fsnotify_get_group(group);
@@ -252,13 +259,12 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
BUG();
}
- 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);
@@ -270,8 +276,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);
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:49 +0200
Subject: [05/10] fanotify: add an extra flag to mark_remove_from_mask that
indicates wheather a mark should be destroyed
commit 6dfbd149946c22c2e2886d6b560def78630c8387 upstream.
This patch adds an extra flag to mark_remove_from_mask() to inform the caller if
the mark should 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 <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/fanotify/fanotify_user.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -513,7 +513,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;
@@ -527,8 +528,7 @@ static __u32 fanotify_mark_remove_from_m
}
spin_unlock(&fsn_mark->lock);
- if (!(oldmask & ~mask))
- fsnotify_destroy_mark(fsn_mark);
+ *destroy = !(oldmask & ~mask);
return mask & oldmask;
}
@@ -539,12 +539,17 @@ static int fanotify_remove_vfsmount_mark
{
struct fsnotify_mark *fsn_mark = NULL;
__u32 removed;
+ int destroy_mark;
fsn_mark = fsnotify_find_vfsmount_mark(group, mnt);
if (!fsn_mark)
return -ENOENT;
- 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);
@@ -558,12 +563,16 @@ static int fanotify_remove_inode_mark(st
{
struct fsnotify_mark *fsn_mark = NULL;
__u32 removed;
+ int destroy_mark;
fsn_mark = fsnotify_find_inode_mark(group, inode);
if (!fsn_mark)
return -ENOENT;
- 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)
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:50 +0200
Subject: [06/10] fsnotify: use a mutex instead of a spinlock to protect a
groups mark list
commit 986ab09807ca9454c3f54aae4db7e1bb00daeed3 upstream.
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 <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/group.c | 2 +-
fs/notify/inode_mark.c | 4 ++--
fs/notify/mark.c | 18 +++++++++---------
fs/notify/vfsmount_mark.c | 4 ++--
include/linux/fsnotify_backend.h | 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/fs/notify/group.c b/fs/notify/group.c
index 354044c..1f73057 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -95,7 +95,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 b13c00a..4e9071e 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -63,8 +63,8 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
{
struct inode *inode = mark->i.inode;
+ BUG_ON(!mutex_is_locked(&mark->group->mark_mutex));
assert_spin_locked(&mark->lock);
- assert_spin_locked(&mark->group->mark_lock);
spin_lock(&inode->i_lock);
@@ -191,8 +191,8 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark,
mark->flags |= FSNOTIFY_MARK_FLAG_INODE;
+ BUG_ON(!mutex_is_locked(&group->mark_mutex));
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 32447dc..ab25b81 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -136,13 +136,13 @@ 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);
/* 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);
goto put_group;
}
@@ -159,7 +159,7 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
list_del_init(&mark->g_list);
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);
@@ -232,11 +232,11 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
/*
* LOCKING ORDER!!!!
- * group->mark_lock
+ * group->mark_mutex
* 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;
@@ -263,7 +263,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);
@@ -277,7 +277,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);
@@ -296,7 +296,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);
@@ -304,7 +304,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 b7b4b0e..f26a348 100644
--- a/fs/notify/vfsmount_mark.c
+++ b/fs/notify/vfsmount_mark.c
@@ -88,8 +88,8 @@ void fsnotify_destroy_vfsmount_mark(struct fsnotify_mark *mark)
{
struct vfsmount *mnt = mark->m.mnt;
+ BUG_ON(!mutex_is_locked(&mark->group->mark_mutex));
assert_spin_locked(&mark->lock);
- assert_spin_locked(&mark->group->mark_lock);
spin_lock(&mnt->mnt_root->d_lock);
@@ -151,8 +151,8 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark,
mark->flags |= FSNOTIFY_MARK_FLAG_VFSMOUNT;
+ BUG_ON(!mutex_is_locked(&group->mark_mutex));
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 e76cef7..c584834 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -141,7 +141,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 */
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:51 +0200
Subject: [07/10] fsnotify: pass group to fsnotify_destroy_mark()
commit e2a29943e9a2ee2aa737a77f550f46ba72269db4 upstream.
In fsnotify_destroy_mark() dont get the group from the passed mark anymore,
but pass the group itself as an additional parameter to the function.
Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/dnotify/dnotify.c | 4 ++--
fs/notify/fanotify/fanotify_user.c | 4 ++--
fs/notify/inode_mark.c | 10 +++++++++-
fs/notify/inotify/inotify_fsnotify.c | 2 +-
fs/notify/inotify/inotify_user.c | 2 +-
fs/notify/mark.c | 21 ++++-----------------
fs/notify/vfsmount_mark.c | 10 +++++++++-
include/linux/fsnotify_backend.h | 5 +++--
kernel/audit_tree.c | 10 +++++-----
kernel/audit_watch.c | 4 ++--
10 files changed, 38 insertions(+), 34 deletions(-)
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -201,7 +201,7 @@ void dnotify_flush(struct file *filp, fl
/* nothing else could have found us thanks to the dnotify_mark_mutex */
if (dn_mark->dn == NULL)
- fsnotify_destroy_mark(fsn_mark);
+ fsnotify_destroy_mark(fsn_mark, dnotify_group);
mutex_unlock(&dnotify_mark_mutex);
@@ -385,7 +385,7 @@ out:
spin_unlock(&fsn_mark->lock);
if (destroy)
- fsnotify_destroy_mark(fsn_mark);
+ fsnotify_destroy_mark(fsn_mark, dnotify_group);
mutex_unlock(&dnotify_mark_mutex);
fsnotify_put_mark(fsn_mark);
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -548,7 +548,7 @@ static int fanotify_remove_vfsmount_mark
removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
&destroy_mark);
if (destroy_mark)
- fsnotify_destroy_mark(fsn_mark);
+ fsnotify_destroy_mark(fsn_mark, group);
fsnotify_put_mark(fsn_mark);
if (removed & mnt->mnt_fsnotify_mask)
@@ -572,7 +572,7 @@ static int fanotify_remove_inode_mark(st
removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
&destroy_mark);
if (destroy_mark)
- fsnotify_destroy_mark(fsn_mark);
+ fsnotify_destroy_mark(fsn_mark, group);
/* matches the fsnotify_find_inode_mark() */
fsnotify_put_mark(fsn_mark);
if (removed & inode->i_fsnotify_mask)
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -99,8 +99,16 @@ void fsnotify_clear_marks_by_inode(struc
spin_unlock(&inode->i_lock);
list_for_each_entry_safe(mark, lmark, &free_list, i.free_i_list) {
- fsnotify_destroy_mark(mark);
+ struct fsnotify_group *group;
+
+ spin_lock(&mark->lock);
+ fsnotify_get_group(mark->group);
+ group = mark->group;
+ spin_unlock(&mark->lock);
+
+ fsnotify_destroy_mark(mark, group);
fsnotify_put_mark(mark);
+ fsnotify_put_group(group);
}
}
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -132,7 +132,7 @@ static int inotify_handle_event(struct f
}
if (inode_mark->mask & IN_ONESHOT)
- fsnotify_destroy_mark(inode_mark);
+ fsnotify_destroy_mark(inode_mark, group);
return ret;
}
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -816,7 +816,7 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, f
ret = 0;
- fsnotify_destroy_mark(&i_mark->fsn_mark);
+ fsnotify_destroy_mark(&i_mark->fsn_mark, group);
/* match ref taken by inotify_idr_find */
fsnotify_put_mark(&i_mark->fsn_mark);
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -122,21 +122,11 @@ EXPORT_SYMBOL_GPL(fsnotify_put_mark);
* The caller had better be holding a reference to this mark so we don't actually
* do the final put under the mark->lock
*/
-void fsnotify_destroy_mark(struct fsnotify_mark *mark)
+void fsnotify_destroy_mark(struct fsnotify_mark *mark,
+ struct fsnotify_group *group)
{
- struct fsnotify_group *group;
struct inode *inode = NULL;
- spin_lock(&mark->lock);
- /* dont get the group from a mark that is not alive yet */
- if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
- spin_unlock(&mark->lock);
- return;
- }
- fsnotify_get_group(mark->group);
- group = mark->group;
- spin_unlock(&mark->lock);
-
mutex_lock(&group->mark_mutex);
spin_lock(&mark->lock);
@@ -144,7 +134,7 @@ void fsnotify_destroy_mark(struct fsnoti
if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) {
spin_unlock(&mark->lock);
mutex_unlock(&group->mark_mutex);
- goto put_group;
+ return;
}
mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
@@ -195,9 +185,6 @@ void fsnotify_destroy_mark(struct fsnoti
*/
atomic_dec(&group->num_marks);
-
-put_group:
- fsnotify_put_group(group);
}
EXPORT_SYMBOL_GPL(fsnotify_destroy_mark);
@@ -310,7 +297,7 @@ void fsnotify_clear_marks_by_group_flags
mutex_unlock(&group->mark_mutex);
list_for_each_entry_safe(mark, lmark, &free_list, free_g_list) {
- fsnotify_destroy_mark(mark);
+ fsnotify_destroy_mark(mark, group);
fsnotify_put_mark(mark);
}
}
--- a/fs/notify/vfsmount_mark.c
+++ b/fs/notify/vfsmount_mark.c
@@ -44,8 +44,16 @@ void fsnotify_clear_marks_by_mount(struc
spin_unlock(&mnt->mnt_root->d_lock);
list_for_each_entry_safe(mark, lmark, &free_list, m.free_m_list) {
- fsnotify_destroy_mark(mark);
+ struct fsnotify_group *group;
+
+ spin_lock(&mark->lock);
+ fsnotify_get_group(mark->group);
+ group = mark->group;
+ spin_unlock(&mark->lock);
+
+ fsnotify_destroy_mark(mark, group);
fsnotify_put_mark(mark);
+ fsnotify_put_group(group);
}
}
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -408,8 +408,9 @@ extern void fsnotify_set_mark_mask_locke
/* 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);
-/* given a mark, flag it to be freed when all references are dropped */
-extern void fsnotify_destroy_mark(struct fsnotify_mark *mark);
+/* given a group and a mark, flag mark to be freed when all references are dropped */
+extern void fsnotify_destroy_mark(struct fsnotify_mark *mark,
+ struct fsnotify_group *group);
/* run all the marks in a group, and clear all of the vfsmount marks */
extern void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group);
/* run all the marks in a group, and clear all of the inode marks */
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -249,7 +249,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_destroy_mark(entry, audit_tree_group);
goto out;
}
@@ -291,7 +291,7 @@ static void untag_chunk(struct node *p)
owner->root = new;
spin_unlock(&hash_lock);
spin_unlock(&entry->lock);
- fsnotify_destroy_mark(entry);
+ fsnotify_destroy_mark(entry, audit_tree_group);
goto out;
Fallback:
@@ -331,7 +331,7 @@ static int create_chunk(struct inode *in
chunk->dead = 1;
spin_unlock(&entry->lock);
fsnotify_get_mark(entry);
- fsnotify_destroy_mark(entry);
+ fsnotify_destroy_mark(entry, audit_tree_group);
fsnotify_put_mark(entry);
return 0;
}
@@ -412,7 +412,7 @@ static int tag_chunk(struct inode *inode
spin_unlock(&old_entry->lock);
fsnotify_get_mark(chunk_entry);
- fsnotify_destroy_mark(chunk_entry);
+ fsnotify_destroy_mark(chunk_entry, audit_tree_group);
fsnotify_put_mark(chunk_entry);
fsnotify_put_mark(old_entry);
@@ -443,7 +443,7 @@ static int tag_chunk(struct inode *inode
spin_unlock(&hash_lock);
spin_unlock(&chunk_entry->lock);
spin_unlock(&old_entry->lock);
- fsnotify_destroy_mark(old_entry);
+ fsnotify_destroy_mark(old_entry, audit_tree_group);
fsnotify_put_mark(old_entry); /* pair to fsnotify_find mark_entry */
return 0;
}
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -349,7 +349,7 @@ static void audit_remove_parent_watches(
}
mutex_unlock(&audit_filter_mutex);
- fsnotify_destroy_mark(&parent->mark);
+ fsnotify_destroy_mark(&parent->mark, audit_watch_group);
}
/* Get path information necessary for adding watches. */
@@ -475,7 +475,7 @@ void audit_remove_watch_rule(struct audi
if (list_empty(&parent->watches)) {
audit_get_parent(parent);
- fsnotify_destroy_mark(&parent->mark);
+ fsnotify_destroy_mark(&parent->mark, audit_watch_group);
audit_put_parent(parent);
}
}
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:52 +0200
Subject: [08/10] fsnotify: introduce locked versions of fsnotify_add_mark()
and fsnotify_remove_mark()
commit d5a335b845792d2a69ed1e244c0b233117b7db3c upstream.
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 <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/mark.c | 42 +++++++++++++++++++++++++++-----------
include/linux/fsnotify_backend.h | 4 ++++
2 files changed, 34 insertions(+), 12 deletions(-)
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -122,18 +122,18 @@ EXPORT_SYMBOL_GPL(fsnotify_put_mark);
* The caller had better be holding a reference to this mark so we don't actually
* do the final put under the mark->lock
*/
-void fsnotify_destroy_mark(struct fsnotify_mark *mark,
- struct fsnotify_group *group)
+void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark,
+ struct fsnotify_group *group)
{
struct inode *inode = NULL;
- mutex_lock(&group->mark_mutex);
+ BUG_ON(!mutex_is_locked(&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);
- mutex_unlock(&group->mark_mutex);
return;
}
@@ -150,6 +150,8 @@ void fsnotify_destroy_mark(struct fsnoti
list_del_init(&mark->g_list);
spin_unlock(&mark->lock);
+
+ /* release lock temporarily */
mutex_unlock(&group->mark_mutex);
spin_lock(&destroy_lock);
@@ -185,6 +187,16 @@ void fsnotify_destroy_mark(struct fsnoti
*/
atomic_dec(&group->num_marks);
+
+ mutex_lock(&group->mark_mutex);
+}
+
+void fsnotify_destroy_mark(struct fsnotify_mark *mark,
+ struct fsnotify_group *group)
+{
+ mutex_lock(&group->mark_mutex);
+ fsnotify_destroy_mark_locked(mark, group);
+ mutex_unlock(&group->mark_mutex);
}
EXPORT_SYMBOL_GPL(fsnotify_destroy_mark);
@@ -210,14 +222,15 @@ void fsnotify_set_mark_ignored_mask_lock
* 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 = 0;
BUG_ON(inode && mnt);
BUG_ON(!inode && !mnt);
+ BUG_ON(!mutex_is_locked(&group->mark_mutex));
/*
* LOCKING ORDER!!!!
@@ -225,8 +238,6 @@ int fsnotify_add_mark(struct fsnotify_ma
* mark->lock
* inode->i_lock
*/
- mutex_lock(&group->mark_mutex);
-
spin_lock(&mark->lock);
mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE;
@@ -252,8 +263,6 @@ int fsnotify_add_mark(struct fsnotify_ma
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);
@@ -266,7 +275,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);
@@ -277,6 +285,16 @@ err:
}
EXPORT_SYMBOL_GPL(fsnotify_add_mark);
+int fsnotify_add_mark(struct fsnotify_mark *mark, struct fsnotify_group *group,
+ struct inode *inode, struct vfsmount *mnt, int allow_dups)
+{
+ int ret;
+ mutex_lock(&group->mark_mutex);
+ ret = fsnotify_add_mark_locked(mark, group, inode, mnt, allow_dups);
+ mutex_unlock(&group->mark_mutex);
+ return ret;
+}
+
/*
* clear any marks in a group in which mark->flags & flags is true
*/
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -408,9 +408,13 @@ extern void fsnotify_set_mark_mask_locke
/* 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 group and a mark, flag mark to be freed when all references are dropped */
extern void fsnotify_destroy_mark(struct fsnotify_mark *mark,
struct fsnotify_group *group);
+extern void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark,
+ struct fsnotify_group *group);
/* run all the marks in a group, and clear all of the vfsmount marks */
extern void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group);
/* run all the marks in a group, and clear all of the inode marks */
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Tue, 14 Jun 2011 17:29:53 +0200
Subject: [09/10] fsnotify: dont put marks on temporary list when clearing
marks by group
commit 64c20d2a20fce295c260ea6cb3b468edfa2fb07b upstream.
In clear_marks_by_group_flags() the mark list of a group is iterated and the
marks are put on a temporary list.
Since we introduced fsnotify_destroy_mark_locked() we dont need the temp list
any more and are able to remove the marks while the mark list is iterated and
the mark list mutex is held.
Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/mark.c | 10 ++--------
include/linux/fsnotify_backend.h | 1 -
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index f9dda03..0e93d90 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -299,22 +299,16 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group,
unsigned int flags)
{
struct fsnotify_mark *lmark, *mark;
- LIST_HEAD(free_list);
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);
- list_del_init(&mark->g_list);
fsnotify_get_mark(mark);
+ fsnotify_destroy_mark_locked(mark, group);
+ fsnotify_put_mark(mark);
}
}
mutex_unlock(&group->mark_mutex);
-
- list_for_each_entry_safe(mark, lmark, &free_list, free_g_list) {
- fsnotify_destroy_mark(mark, group);
- fsnotify_put_mark(mark);
- }
}
/*
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 26c06af..5a88993 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -287,7 +287,6 @@ struct fsnotify_mark {
struct fsnotify_inode_mark i;
struct fsnotify_vfsmount_mark m;
};
- struct list_head free_g_list; /* tmp list used when freeing this mark */
__u32 ignored_mask; /* events types to ignore */
#define FSNOTIFY_MARK_FLAG_INODE 0x01
#define FSNOTIFY_MARK_FLAG_VFSMOUNT 0x02
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Fri, 12 Aug 2011 01:13:31 +0200
Subject: [10/10] fsnotify: change locking order
commit 6960b0d909cde5bdff49e4e5c1250edd10be7ebd upstream.
On Mon, Aug 01, 2011 at 04:38:22PM -0400, Eric Paris wrote:
>
> I finally built and tested a v3.0 kernel with these patches (I know I'm
> SOOOOOO far behind). Not what I hoped for:
>
> > [ 150.937798] VFS: Busy inodes after unmount of tmpfs. Self-destruct in 5 seconds. Have a nice day...
> > [ 150.945290] BUG: unable to handle kernel NULL pointer dereference at 0000000000000070
> > [ 150.946012] IP: [<ffffffff810ffd58>] shmem_free_inode+0x18/0x50
> > [ 150.946012] PGD 2bf9e067 PUD 2bf9f067 PMD 0
> > [ 150.946012] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> > [ 150.946012] CPU 0
> > [ 150.946012] Modules linked in: nfs lockd fscache auth_rpcgss nfs_acl sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables ext4 jbd2 crc16 joydev ata_piix i2c_piix4 pcspkr uinput ipv6 autofs4 usbhid [last unloaded: scsi_wait_scan]
> > [ 150.946012]
> > [ 150.946012] Pid: 2764, comm: syscall_thrash Not tainted 3.0.0+ #1 Red Hat KVM
> > [ 150.946012] RIP: 0010:[<ffffffff810ffd58>] [<ffffffff810ffd58>] shmem_free_inode+0x18/0x50
> > [ 150.946012] RSP: 0018:ffff88002c2e5df8 EFLAGS: 00010282
> > [ 150.946012] RAX: 000000004e370d9f RBX: 0000000000000000 RCX: ffff88003a029438
> > [ 150.946012] RDX: 0000000033630a5f RSI: 0000000000000000 RDI: ffff88003491c240
> > [ 150.946012] RBP: ffff88002c2e5e08 R08: 0000000000000000 R09: 0000000000000000
> > [ 150.946012] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88003a029428
> > [ 150.946012] R13: ffff88003a029428 R14: ffff88003a029428 R15: ffff88003499a610
> > [ 150.946012] FS: 00007f5a05420700(0000) GS:ffff88003f600000(0000) knlGS:0000000000000000
> > [ 150.946012] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > [ 150.946012] CR2: 0000000000000070 CR3: 000000002a662000 CR4: 00000000000006f0
> > [ 150.946012] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 150.946012] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > [ 150.946012] Process syscall_thrash (pid: 2764, threadinfo ffff88002c2e4000, task ffff88002bfbc760)
> > [ 150.946012] Stack:
> > [ 150.946012] ffff88003a029438 ffff88003a029428 ffff88002c2e5e38 ffffffff81102f76
> > [ 150.946012] ffff88003a029438 ffff88003a029598 ffffffff8160f9c0 ffff88002c221250
> > [ 150.946012] ffff88002c2e5e68 ffffffff8115e9be ffff88002c2e5e68 ffff88003a029438
> > [ 150.946012] Call Trace:
> > [ 150.946012] [<ffffffff81102f76>] shmem_evict_inode+0x76/0x130
> > [ 150.946012] [<ffffffff8115e9be>] evict+0x7e/0x170
> > [ 150.946012] [<ffffffff8115ee40>] iput_final+0xd0/0x190
> > [ 150.946012] [<ffffffff8115ef33>] iput+0x33/0x40
> > [ 150.946012] [<ffffffff81180205>] fsnotify_destroy_mark_locked+0x145/0x160
> > [ 150.946012] [<ffffffff81180316>] fsnotify_destroy_mark+0x36/0x50
> > [ 150.946012] [<ffffffff81181937>] sys_inotify_rm_watch+0x77/0xd0
> > [ 150.946012] [<ffffffff815aca52>] system_call_fastpath+0x16/0x1b
> > [ 150.946012] Code: 67 4a 00 b8 e4 ff ff ff eb aa 66 0f 1f 84 00 00 00 00 00 55 48 89 e5 48 83 ec 10 48 89 1c 24 4c 89 64 24 08 48 8b 9f 40 05 00 00
> > [ 150.946012] 83 7b 70 00 74 1c 4c 8d a3 80 00 00 00 4c 89 e7 e8 d2 5d 4a
> > [ 150.946012] RIP [<ffffffff810ffd58>] shmem_free_inode+0x18/0x50
> > [ 150.946012] RSP <ffff88002c2e5df8>
> > [ 150.946012] CR2: 0000000000000070
>
> Looks at aweful lot like the problem from:
> http://www.spinics.net/lists/linux-fsdevel/msg46101.html
>
I tried to reproduce this bug with your test program, but without success.
However, if I understand correctly, this occurs since we dont hold any locks when
we call iput() in mark_destroy(), right?
With the patches you tested, iput() is also not called within any lock, since the
groups mark_mutex is released temporarily before iput() is called. This is, since
the original codes behaviour is similar.
However since we now have a mutex as the biggest lock, we can do what you
suggested (http://www.spinics.net/lists/linux-fsdevel/msg46107.html) and
call iput() with the mutex held to avoid the race.
The patch below implements this. It uses nested locking to avoid deadlock in case
we do the final iput() on an inode which still holds marks and thus would take
the mutex again when calling fsnotify_inode_delete() in destroy_inode().
Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/mark.c | 20 ++++++++++----------
include/linux/fsnotify_backend.h | 7 ++++---
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 0e93d90..fc6b49b 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -150,6 +150,8 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark,
spin_unlock(&mark->lock);
+ if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED))
+ iput(inode);
/* release lock temporarily */
mutex_unlock(&group->mark_mutex);
@@ -157,6 +159,11 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark,
list_add(&mark->destroy_list, &destroy_list);
spin_unlock(&destroy_lock);
wake_up(&destroy_waitq);
+ /*
+ * We don't necessarily have a ref on mark from caller so the above destroy
+ * may have actually freed it, unless this group provides a 'freeing_mark'
+ * function which must be holding a reference.
+ */
/*
* Some groups like to know that marks are being freed. This is a
@@ -178,22 +185,15 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark,
* is just a lazy update (and could be a perf win...)
*/
- if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED))
- iput(inode);
- /*
- * We don't necessarily have a ref on mark from caller so the above iput
- * may have already destroyed it. Don't touch from now on.
- */
-
atomic_dec(&group->num_marks);
- mutex_lock(&group->mark_mutex);
+ mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING);
}
void fsnotify_destroy_mark(struct fsnotify_mark *mark,
struct fsnotify_group *group)
{
- mutex_lock(&group->mark_mutex);
+ mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING);
fsnotify_destroy_mark_locked(mark, group);
mutex_unlock(&group->mark_mutex);
}
@@ -300,7 +300,7 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group,
{
struct fsnotify_mark *lmark, *mark;
- mutex_lock(&group->mark_mutex);
+ mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING);
list_for_each_entry_safe(mark, lmark, &group->marks_list, g_list) {
if (mark->flags & flags) {
fsnotify_get_mark(mark);
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 5a88993..1af2f6a 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -88,9 +88,10 @@ struct fsnotify_event_private_data;
* if the group is interested in this event.
* handle_event - main call for a group to handle an fs event
* free_group_priv - called when a group refcnt hits 0 to clean up the private union
- * freeing-mark - this means that a mark has been flagged to die when everything
- * finishes using it. The function is supplied with what must be a
- * valid group and inode to use to clean up.
+ * freeing_mark - called when a mark is being destroyed for some reason. The group
+ * MUST be holding a reference on each mark and that reference must be
+ * dropped in this function. inotify uses this function to send
+ * userspace messages that marks have been removed.
*/
struct fsnotify_ops {
bool (*should_send_event)(struct fsnotify_group *group, struct inode *inode,
Attachment:
signature.asc
Description: This is a digitally signed message part