fsnotify: use reference counting for groups

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>

diff --git a/fs/notify/group.c b/fs/notify/group.c
index 1d57c35f10431b617dc39c6f7eedc2af18cd5cba..354044c47e23f23800bb34aad9e46b4edebc2418 100644 (file)
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -33,9 +33,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);
 
@@ -43,12 +40,10 @@ void fsnotify_final_destroy_group(struct fsnotify_group *group)
 }
 
 /*
- * 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)
 {
@@ -57,9 +52,10 @@ void fsnotify_destroy_group(struct fsnotify_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);
+       /* clear the notification queue of all events */
+       fsnotify_flush_notify(group);
+
+       fsnotify_put_group(group);
 }
 
 /*
@@ -76,7 +72,7 @@ void fsnotify_get_group(struct fsnotify_group *group)
 void fsnotify_put_group(struct fsnotify_group *group)
 {
        if (atomic_dec_and_test(&group->refcnt))
-               fsnotify_destroy_group(group);
+               fsnotify_final_destroy_group(group);
 }
 
 /*
@@ -92,11 +88,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->notification_mutex);
        INIT_LIST_HEAD(&group->notification_list);

diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index e3cbd746f64a5eb4ac9359cf8a3f271a6da8a332..74977fbf5aae9e08bec87cbb3d9dc0d6ded2bfb0 100644 (file)
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -118,6 +118,7 @@ static int inotify_handle_event(struct fsnotify_group *group,
 
        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 fsnotify_event_private_data *fsn_event_priv)
        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);
 }
 

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index dbafbfc8ceca86040ba30b6d3d18cef81688f828..246250f1db7ad8817cca2160c68933ec27d6a75e 100644 (file)
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -531,6 +531,7 @@ void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
 
        fsn_event_priv = &event_priv->fsnotify_event_priv_data;
 
+       fsnotify_get_group(group);
        fsn_event_priv->group = group;
        event_priv->wd = i_mark->wd;
 

diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index f104d565b6823aa75ab39c5c0717e9e3fd9382f3..3c7a1699df3d0f9080d761f28512292bf5cfa552 100644 (file)
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -109,8 +109,11 @@ void fsnotify_get_mark(struct fsnotify_mark *mark)
 
 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);
+       }
 }
 
 /*
@@ -125,12 +128,13 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
 
        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;
@@ -177,19 +181,15 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark)
 
        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);
 }
 
 void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask)
@@ -234,6 +234,7 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
 
        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);
@@ -265,6 +266,7 @@ int fsnotify_add_mark(struct fsnotify_mark *mark,
 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);
 
@@ -317,6 +319,8 @@ void fsnotify_duplicate_mark(struct fsnotify_mark *new, struct fsnotify_mark *ol
        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;