diff mbox

[v2,6/7] fsnotify: clean up fsnotify()

Message ID 1508920899-8115-7-git-send-email-mszeredi@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Miklos Szeredi Oct. 25, 2017, 8:41 a.m. UTC 
Use helpers to get first and next marks from connector.

Also get rid of inode_node/vfsmount_node local variables, which just refers
to the same objects as iter_info.  There was an srcu_dereference() for
foo_node, but that's completely superfluous since we've already done it
when obtaining foo_node.

Also get rid of inode_group/vfsmount_group local variables; checking
against non-NULL for these is the same as checking against non-NULL
inode_mark/vfsmount_mark.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/notify/fsnotify.c | 113 ++++++++++++++++++++++++---------------------------
 1 file changed, 54 insertions(+), 59 deletions(-)

Comments

Amir Goldstein Oct. 25, 2017, 11:46 a.m. UTC | #1 
On Wed, Oct 25, 2017 at 11:41 AM, Miklos Szeredi <mszeredi@redhat.com> wrote:
> Use helpers to get first and next marks from connector.
>
> Also get rid of inode_node/vfsmount_node local variables, which just refers
> to the same objects as iter_info.  There was an srcu_dereference() for
> foo_node, but that's completely superfluous since we've already done it
> when obtaining foo_node.
>
> Also get rid of inode_group/vfsmount_group local variables; checking
> against non-NULL for these is the same as checking against non-NULL
> inode_mark/vfsmount_mark.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/notify/fsnotify.c | 113 ++++++++++++++++++++++++---------------------------
>  1 file changed, 54 insertions(+), 59 deletions(-)
>
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 0ab6a7179e4d..dcd70e94cbf1 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -243,6 +243,29 @@ static int send_to_group(struct inode *to_tell,
>                                         file_name, cookie, iter_info);
>  }
>
> +static struct fsnotify_mark *fsnotify_first_mark(struct fsnotify_mark_connector **connp)
> +{
> +       struct fsnotify_mark_connector *conn;
> +       struct hlist_node *node = NULL;
> +
> +       conn = srcu_dereference(*connp, &fsnotify_mark_srcu);
> +       if (conn)
> +               node = srcu_dereference(conn->list.first, &fsnotify_mark_srcu);
> +
> +       return hlist_entry_safe(node, struct fsnotify_mark, obj_list);
> +}
> +
> +static struct fsnotify_mark *fsnotify_next_mark(struct fsnotify_mark *mark)
> +{
> +       struct hlist_node *node = NULL;
> +
> +       if (mark)
> +               node = srcu_dereference(mark->obj_list.next,
> +                                       &fsnotify_mark_srcu);
> +
> +       return hlist_entry_safe(node, struct fsnotify_mark, obj_list);
> +}
> +
>  /*
>   * This is the main call to fsnotify.  The VFS calls into hook specific functions
>   * in linux/fsnotify.h.  Those functions then in turn call here.  Here will call
> @@ -252,11 +275,7 @@ static int send_to_group(struct inode *to_tell,
>  int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>              const unsigned char *file_name, u32 cookie)
>  {
> -       struct hlist_node *inode_node = NULL, *vfsmount_node = NULL;
> -       struct fsnotify_mark *inode_mark = NULL, *vfsmount_mark = NULL;
> -       struct fsnotify_group *inode_group, *vfsmount_group;
> -       struct fsnotify_mark_connector *inode_conn, *vfsmount_conn;
> -       struct fsnotify_iter_info iter_info;
> +       struct fsnotify_iter_info iter_info = { NULL, NULL };

Forgot to change to = { };

>         struct mount *mnt;
>         int ret = 0;
>         /* global tests shouldn't care about events on child only the specific event */
> @@ -291,26 +310,16 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>
>         if ((mask & FS_MODIFY) ||
>             (test_mask & to_tell->i_fsnotify_mask)) {
> -               inode_conn = srcu_dereference(to_tell->i_fsnotify_marks,
> -                                             &fsnotify_mark_srcu);
> -               if (inode_conn)
> -                       inode_node = srcu_dereference(inode_conn->list.first,
> -                                                     &fsnotify_mark_srcu);
> +               iter_info.inode_mark =
> +                       fsnotify_first_mark(&to_tell->i_fsnotify_marks);
>         }
>
>         if (mnt && ((mask & FS_MODIFY) ||
>                     (test_mask & mnt->mnt_fsnotify_mask))) {
> -               inode_conn = srcu_dereference(to_tell->i_fsnotify_marks,
> -                                             &fsnotify_mark_srcu);
> -               if (inode_conn)
> -                       inode_node = srcu_dereference(inode_conn->list.first,
> -                                                     &fsnotify_mark_srcu);
> -               vfsmount_conn = srcu_dereference(mnt->mnt_fsnotify_marks,
> -                                                &fsnotify_mark_srcu);
> -               if (vfsmount_conn)
> -                       vfsmount_node = srcu_dereference(
> -                                               vfsmount_conn->list.first,
> -                                               &fsnotify_mark_srcu);
> +               iter_info.inode_mark =
> +                       fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> +               iter_info.vfsmount_mark =
> +                       fsnotify_first_mark(&mnt->mnt_fsnotify_marks);
>         }
>
>         /*
> @@ -318,41 +327,28 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>          * ignore masks are properly reflected for mount mark notifications.
>          * That's why this traversal is so complicated...
>          */
> -       while (inode_node || vfsmount_node) {
> -               inode_group = NULL;
> -               inode_mark = NULL;
> -               vfsmount_group = NULL;
> -               vfsmount_mark = NULL;
> -
> -               if (inode_node) {
> -                       inode_mark = hlist_entry(srcu_dereference(inode_node, &fsnotify_mark_srcu),
> -                                                struct fsnotify_mark, obj_list);
> -                       inode_group = inode_mark->group;
> -                       if (!(inode_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED))
> -                               goto skip_inode;
> -               }
> +       while (iter_info.inode_mark || iter_info.vfsmount_mark) {
> +               struct fsnotify_mark *inode_mark = iter_info.inode_mark;
> +               struct fsnotify_mark *vfsmount_mark = iter_info.vfsmount_mark;
>
> -               if (vfsmount_node) {
> -                       vfsmount_mark = hlist_entry(srcu_dereference(vfsmount_node, &fsnotify_mark_srcu),
> -                                                   struct fsnotify_mark, obj_list);
> -                       vfsmount_group = vfsmount_mark->group;
> -                       if (!(vfsmount_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED))
> -                               goto skip_vfsmount;
> +               if (inode_mark &&
> +                   !(inode_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) {
> +                       vfsmount_mark = NULL;
> +                       goto skip;
> +               }
> +               if (vfsmount_mark &&
> +                   !(vfsmount_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) {
> +                       inode_mark = NULL;
> +                       goto skip;
>                 }
>
> -               iter_info.inode_mark = inode_mark;
> -               iter_info.vfsmount_mark = vfsmount_mark;
> -
> -               if (inode_group && vfsmount_group) {
> -                       int cmp = fsnotify_compare_groups(inode_group,
> -                                                         vfsmount_group);
> -                       if (cmp > 0) {
> -                               inode_group = NULL;
> +               if (inode_mark && vfsmount_mark) {
> +                       int cmp = fsnotify_compare_groups(inode_mark->group,
> +                                                         vfsmount_mark->group);
> +                       if (cmp > 0)
>                                 inode_mark = NULL;
> -                       } else if (cmp < 0) {
> -                               vfsmount_group = NULL;
> +                       else if (cmp < 0)
>                                 vfsmount_mark = NULL;
> -                       }
>                 }
>
>                 ret = send_to_group(to_tell, inode_mark, vfsmount_mark, mask,
> @@ -361,14 +357,13 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>
>                 if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS))
>                         goto out;
> -skip_inode:
> -               if (inode_group)
> -                       inode_node = srcu_dereference(inode_node->next,
> -                                                     &fsnotify_mark_srcu);
> -skip_vfsmount:
> -               if (vfsmount_group)
> -                       vfsmount_node = srcu_dereference(vfsmount_node->next,
> -                                                        &fsnotify_mark_srcu);
> +skip:
> +               if (inode_mark)
> +                       iter_info.inode_mark =
> +                               fsnotify_next_mark(iter_info.inode_mark);
> +               if (vfsmount_mark)
> +                       iter_info.vfsmount_mark =
> +                               fsnotify_next_mark(iter_info.vfsmount_mark);
>         }
>         ret = 0;
>  out:
> --
> 2.5.5
>
diff mbox

Patch

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 0ab6a7179e4d..dcd70e94cbf1 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -243,6 +243,29 @@  static int send_to_group(struct inode *to_tell,
 					file_name, cookie, iter_info);
 }
 
+static struct fsnotify_mark *fsnotify_first_mark(struct fsnotify_mark_connector **connp)
+{
+	struct fsnotify_mark_connector *conn;
+	struct hlist_node *node = NULL;
+
+	conn = srcu_dereference(*connp, &fsnotify_mark_srcu);
+	if (conn)
+		node = srcu_dereference(conn->list.first, &fsnotify_mark_srcu);
+
+	return hlist_entry_safe(node, struct fsnotify_mark, obj_list);
+}
+
+static struct fsnotify_mark *fsnotify_next_mark(struct fsnotify_mark *mark)
+{
+	struct hlist_node *node = NULL;
+
+	if (mark)
+		node = srcu_dereference(mark->obj_list.next,
+					&fsnotify_mark_srcu);
+
+	return hlist_entry_safe(node, struct fsnotify_mark, obj_list);
+}
+
 /*
  * This is the main call to fsnotify.  The VFS calls into hook specific functions
  * in linux/fsnotify.h.  Those functions then in turn call here.  Here will call
@@ -252,11 +275,7 @@  static int send_to_group(struct inode *to_tell,
 int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 	     const unsigned char *file_name, u32 cookie)
 {
-	struct hlist_node *inode_node = NULL, *vfsmount_node = NULL;
-	struct fsnotify_mark *inode_mark = NULL, *vfsmount_mark = NULL;
-	struct fsnotify_group *inode_group, *vfsmount_group;
-	struct fsnotify_mark_connector *inode_conn, *vfsmount_conn;
-	struct fsnotify_iter_info iter_info;
+	struct fsnotify_iter_info iter_info = { NULL, NULL };
 	struct mount *mnt;
 	int ret = 0;
 	/* global tests shouldn't care about events on child only the specific event */
@@ -291,26 +310,16 @@  int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 
 	if ((mask & FS_MODIFY) ||
 	    (test_mask & to_tell->i_fsnotify_mask)) {
-		inode_conn = srcu_dereference(to_tell->i_fsnotify_marks,
-					      &fsnotify_mark_srcu);
-		if (inode_conn)
-			inode_node = srcu_dereference(inode_conn->list.first,
-						      &fsnotify_mark_srcu);
+		iter_info.inode_mark =
+			fsnotify_first_mark(&to_tell->i_fsnotify_marks);
 	}
 
 	if (mnt && ((mask & FS_MODIFY) ||
 		    (test_mask & mnt->mnt_fsnotify_mask))) {
-		inode_conn = srcu_dereference(to_tell->i_fsnotify_marks,
-					      &fsnotify_mark_srcu);
-		if (inode_conn)
-			inode_node = srcu_dereference(inode_conn->list.first,
-						      &fsnotify_mark_srcu);
-		vfsmount_conn = srcu_dereference(mnt->mnt_fsnotify_marks,
-					         &fsnotify_mark_srcu);
-		if (vfsmount_conn)
-			vfsmount_node = srcu_dereference(
-						vfsmount_conn->list.first,
-						&fsnotify_mark_srcu);
+		iter_info.inode_mark =
+			fsnotify_first_mark(&to_tell->i_fsnotify_marks);
+		iter_info.vfsmount_mark =
+			fsnotify_first_mark(&mnt->mnt_fsnotify_marks);
 	}
 
 	/*
@@ -318,41 +327,28 @@  int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 	 * ignore masks are properly reflected for mount mark notifications.
 	 * That's why this traversal is so complicated...
 	 */
-	while (inode_node || vfsmount_node) {
-		inode_group = NULL;
-		inode_mark = NULL;
-		vfsmount_group = NULL;
-		vfsmount_mark = NULL;
-
-		if (inode_node) {
-			inode_mark = hlist_entry(srcu_dereference(inode_node, &fsnotify_mark_srcu),
-						 struct fsnotify_mark, obj_list);
-			inode_group = inode_mark->group;
-			if (!(inode_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED))
-				goto skip_inode;
-		}
+	while (iter_info.inode_mark || iter_info.vfsmount_mark) {
+		struct fsnotify_mark *inode_mark = iter_info.inode_mark;
+		struct fsnotify_mark *vfsmount_mark = iter_info.vfsmount_mark;
 
-		if (vfsmount_node) {
-			vfsmount_mark = hlist_entry(srcu_dereference(vfsmount_node, &fsnotify_mark_srcu),
-						    struct fsnotify_mark, obj_list);
-			vfsmount_group = vfsmount_mark->group;
-			if (!(vfsmount_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED))
-				goto skip_vfsmount;
+		if (inode_mark &&
+		    !(inode_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) {
+			vfsmount_mark = NULL;
+			goto skip;
+		}
+		if (vfsmount_mark &&
+		    !(vfsmount_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) {
+			inode_mark = NULL;
+			goto skip;
 		}
 
-		iter_info.inode_mark = inode_mark;
-		iter_info.vfsmount_mark = vfsmount_mark;
-
-		if (inode_group && vfsmount_group) {
-			int cmp = fsnotify_compare_groups(inode_group,
-							  vfsmount_group);
-			if (cmp > 0) {
-				inode_group = NULL;
+		if (inode_mark && vfsmount_mark) {
+			int cmp = fsnotify_compare_groups(inode_mark->group,
+							  vfsmount_mark->group);
+			if (cmp > 0)
 				inode_mark = NULL;
-			} else if (cmp < 0) {
-				vfsmount_group = NULL;
+			else if (cmp < 0)
 				vfsmount_mark = NULL;
-			}
 		}
 
 		ret = send_to_group(to_tell, inode_mark, vfsmount_mark, mask,
@@ -361,14 +357,13 @@  int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 
 		if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS))
 			goto out;
-skip_inode:
-		if (inode_group)
-			inode_node = srcu_dereference(inode_node->next,
-						      &fsnotify_mark_srcu);
-skip_vfsmount:
-		if (vfsmount_group)
-			vfsmount_node = srcu_dereference(vfsmount_node->next,
-							 &fsnotify_mark_srcu);
+skip:
+		if (inode_mark)
+			iter_info.inode_mark =
+				fsnotify_next_mark(iter_info.inode_mark);
+		if (vfsmount_mark)
+			iter_info.vfsmount_mark =
+				fsnotify_next_mark(iter_info.vfsmount_mark);
 	}
 	ret = 0;
 out: