From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932197AbdJYLqX (ORCPT ); Wed, 25 Oct 2017 07:46:23 -0400 Received: from mail-yw0-f195.google.com ([209.85.161.195]:50222 "EHLO mail-yw0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750852AbdJYLqV (ORCPT ); Wed, 25 Oct 2017 07:46:21 -0400 X-Google-Smtp-Source: ABhQp+RCB2Klv1Hs56eHQZKMAHmcSALCvz82+rSdJ2aBfI8mib0NuCX0Zz9JO1bkWWWhLqO/35Kw+QblUqe4M+X/uiA= MIME-Version: 1.0 In-Reply-To: <1508920899-8115-7-git-send-email-mszeredi@redhat.com> References: <1508920899-8115-1-git-send-email-mszeredi@redhat.com> <1508920899-8115-7-git-send-email-mszeredi@redhat.com> From: Amir Goldstein Date: Wed, 25 Oct 2017 14:46:20 +0300 Message-ID: Subject: Re: [PATCH v2 6/7] fsnotify: clean up fsnotify() To: Miklos Szeredi Cc: linux-fsdevel , Jan Kara , Xiong Zhou , linux-kernel Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 25, 2017 at 11:41 AM, Miklos Szeredi 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 > --- > 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 >