From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:51350 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934109AbcLVPQb (ORCPT ); Thu, 22 Dec 2016 10:16:31 -0500 Date: Thu, 22 Dec 2016 16:16:27 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , linux-fsdevel , Lino Sanfilippo , Miklos Szeredi , Paul Moore Subject: Re: [PATCH 01/22] fsnotify: Remove unnecessary tests when showing fdinfo Message-ID: <20161222151627.GG28510@quack2.suse.cz> References: <20161222091538.28702-1-jack@suse.cz> <20161222091538.28702-2-jack@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu 22-12-16 14:59:35, Amir Goldstein wrote: > On Thu, Dec 22, 2016 at 11:15 AM, Jan Kara wrote: > > show_fdinfo() iterates group's list of marks. All marks found there are > > guaranteed to be alive and they stay so until we release > > group->mark_mutex. So remove uncecessary tests whether mark is alive. > > > > The statement above is true for fanotify. I don't think it holds for inotify. > > SYS_inotify_rm_watch() > fsnotify_destroy_mark() > fsnotify_destroy_mark(mark, group) > mutex_lock_nested(&group->mark_mutex) /* not really nested for > inotify */ > fsnotify_detach_mark(mark) > mutex_unlock(&group->mark_mutex); > fsnotify_free_mark(mark) > mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE; > /* !!! mark is not alive and on the group's list. group->mark_mutex is > not held !!! */ How come it is on the group's list? fsnotify_detach_mark() will remove it from that list... The destroy_list is just a private list used for mark destruction, not a list of any group. > list_add(&mark->g_list, &destroy_list); Honza > > > Signed-off-by: Jan Kara > > --- > > fs/notify/fdinfo.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c > > index fd98e5100cab..601a59c8d87e 100644 > > --- a/fs/notify/fdinfo.c > > +++ b/fs/notify/fdinfo.c > > @@ -76,8 +76,7 @@ static void inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) > > struct inotify_inode_mark *inode_mark; > > struct inode *inode; > > > > - if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE) || > > - !(mark->flags & FSNOTIFY_MARK_FLAG_INODE)) > > + if (!(mark->flags & FSNOTIFY_MARK_FLAG_INODE)) > > return; > > > > inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark); > > @@ -113,9 +112,6 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) > > unsigned int mflags = 0; > > struct inode *inode; > > > > - if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) > > - return; > > - > > if (mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY) > > mflags |= FAN_MARK_IGNORED_SURV_MODIFY; > > > > -- > > 2.10.2 > > -- Jan Kara SUSE Labs, CR