fsnotify: Make fsnotify_mark_connector hold inode reference
authorJan Kara <jack@suse.cz>
Tue, 14 Mar 2017 13:48:00 +0000 (14:48 +0100)
committerJan Kara <jack@suse.cz>
Mon, 10 Apr 2017 15:37:35 +0000 (17:37 +0200)
Currently inode reference is held by fsnotify marks. Change the rules so
that inode reference is held by fsnotify_mark_connector structure
whenever the list is non-empty. This simplifies the code and is more
logical.

Reviewed-by: Miklos Szeredi <mszeredi@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
fs/notify/fsnotify.h
fs/notify/inode_mark.c
fs/notify/mark.c
include/linux/fsnotify_backend.h

index dd1a679..1a2aec6 100644 (file)
@@ -21,8 +21,6 @@ extern u32 fsnotify_recalc_mask(struct fsnotify_mark_connector *conn);
 extern int fsnotify_compare_groups(struct fsnotify_group *a,
                                   struct fsnotify_group *b);
 
-extern void fsnotify_set_inode_mark_mask_locked(struct fsnotify_mark *fsn_mark,
-                                               __u32 mask);
 /* Add mark to a proper place in mark list */
 extern int fsnotify_add_mark_list(struct fsnotify_mark_connector **connp,
                                  struct fsnotify_mark *mark,
@@ -40,7 +38,7 @@ extern int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark,
 /* vfsmount specific destruction of a mark */
 extern void fsnotify_destroy_vfsmount_mark(struct fsnotify_mark *mark);
 /* inode specific destruction of a mark */
-extern void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark);
+extern struct inode *fsnotify_destroy_inode_mark(struct fsnotify_mark *mark);
 /* Find mark belonging to given group in the list of marks */
 extern struct fsnotify_mark *fsnotify_find_mark(
                                        struct fsnotify_mark_connector *conn,
index 1644ba0..c3873b6 100644 (file)
@@ -43,9 +43,10 @@ void fsnotify_recalc_inode_mask(struct inode *inode)
        __fsnotify_update_child_dentry_flags(inode);
 }
 
-void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
+struct inode *fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
 {
        struct inode *inode = mark->connector->inode;
+       bool empty;
 
        BUG_ON(!mutex_is_locked(&mark->group->mark_mutex));
        assert_spin_locked(&mark->lock);
@@ -53,6 +54,7 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
        spin_lock(&inode->i_lock);
 
        hlist_del_init_rcu(&mark->obj_list);
+       empty = hlist_empty(&mark->connector->list);
        mark->connector = NULL;
 
        /*
@@ -62,6 +64,8 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
         */
        inode->i_fsnotify_mask = fsnotify_recalc_mask(inode->i_fsnotify_marks);
        spin_unlock(&inode->i_lock);
+
+       return empty ? inode : NULL;
 }
 
 /*
@@ -89,30 +93,6 @@ struct fsnotify_mark *fsnotify_find_inode_mark(struct fsnotify_group *group,
 }
 
 /*
- * If we are setting a mark mask on an inode mark we should pin the inode
- * in memory.
- */
-void fsnotify_set_inode_mark_mask_locked(struct fsnotify_mark *mark,
-                                        __u32 mask)
-{
-       struct inode *inode;
-
-       assert_spin_locked(&mark->lock);
-
-       if (mask && mark->connector &&
-           !(mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED)) {
-               mark->flags |= FSNOTIFY_MARK_FLAG_OBJECT_PINNED;
-               inode = igrab(mark->connector->inode);
-               /*
-                * we shouldn't be able to get here if the inode wasn't
-                * already safely held in memory.  But bug in case it
-                * ever is wrong.
-                */
-               BUG_ON(!inode);
-       }
-}
-
-/*
  * Attach an initialized mark to a given inode.
  * These marks may be used for the fsnotify backend to determine which
  * event types should be delivered to which group and for which inodes.  These
index 3d6e7a8..8a15c64 100644 (file)
@@ -142,10 +142,9 @@ void fsnotify_detach_mark(struct fsnotify_mark *mark)
 
        mark->flags &= ~FSNOTIFY_MARK_FLAG_ATTACHED;
 
-       if (mark->connector->flags & FSNOTIFY_OBJ_TYPE_INODE) {
-               inode = mark->connector->inode;
-               fsnotify_destroy_inode_mark(mark);
-       } else if (mark->connector->flags & FSNOTIFY_OBJ_TYPE_VFSMOUNT)
+       if (mark->connector->flags & FSNOTIFY_OBJ_TYPE_INODE)
+               inode = fsnotify_destroy_inode_mark(mark);
+       else if (mark->connector->flags & FSNOTIFY_OBJ_TYPE_VFSMOUNT)
                fsnotify_destroy_vfsmount_mark(mark);
        else
                BUG();
@@ -160,7 +159,7 @@ void fsnotify_detach_mark(struct fsnotify_mark *mark)
 
        spin_unlock(&mark->lock);
 
-       if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED))
+       if (inode)
                iput(inode);
 
        atomic_dec(&group->num_marks);
@@ -274,9 +273,6 @@ void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask)
        assert_spin_locked(&mark->lock);
 
        mark->mask = mask;
-
-       if (mark->connector && mark->connector->flags & FSNOTIFY_OBJ_TYPE_INODE)
-               fsnotify_set_inode_mark_mask_locked(mark, mask);
 }
 
 void fsnotify_set_mark_ignored_mask_locked(struct fsnotify_mark *mark, __u32 mask)
@@ -375,6 +371,8 @@ int fsnotify_add_mark_list(struct fsnotify_mark_connector **connp,
        /* is mark the first mark? */
        if (hlist_empty(&conn->list)) {
                hlist_add_head_rcu(&mark->obj_list, &conn->list);
+               if (inode)
+                       __iget(inode);
                goto added;
        }
 
@@ -441,9 +439,6 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
        } else {
                BUG();
        }
-
-       /* this will pin the object if appropriate */
-       fsnotify_set_mark_mask_locked(mark, mark->mask);
        spin_unlock(&mark->lock);
 
        if (inode)
index 06f9a2c..96333fb 100644 (file)
@@ -196,8 +196,9 @@ struct fsnotify_group {
 
 /*
  * Inode / vfsmount point to this structure which tracks all marks attached to
- * the inode / vfsmount. The structure is freed only when inode / vfsmount gets
- * freed.
+ * the inode / vfsmount. The reference to inode / vfsmount is held by this
+ * structure whenever the list is non-empty. The structure is freed only when
+ * inode / vfsmount gets freed.
  */
 struct fsnotify_mark_connector {
 #define FSNOTIFY_OBJ_TYPE_INODE                0x01
@@ -245,10 +246,9 @@ struct fsnotify_mark {
        struct fsnotify_mark_connector *connector;
        /* Events types to ignore [mark->lock, group->mark_mutex] */
        __u32 ignored_mask;
-#define FSNOTIFY_MARK_FLAG_OBJECT_PINNED       0x04
-#define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x08
-#define FSNOTIFY_MARK_FLAG_ALIVE               0x10
-#define FSNOTIFY_MARK_FLAG_ATTACHED            0x20
+#define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x01
+#define FSNOTIFY_MARK_FLAG_ALIVE               0x02
+#define FSNOTIFY_MARK_FLAG_ATTACHED            0x04
        unsigned int flags;             /* flags [mark->lock] */
        void (*free_mark)(struct fsnotify_mark *mark); /* called on final put+free */
 };