From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:53256 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727433AbeHaSEP (ORCPT ); Fri, 31 Aug 2018 14:04:15 -0400 Date: Fri, 31 Aug 2018 15:50:20 +0200 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Marko Rauhamaa , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v3 2/3] fsnotify: send path type events to group with super block marks Message-ID: <20180831135020.GI11622@quack2.suse.cz> References: <20180830151551.27422-1-amir73il@gmail.com> <20180830151551.27422-3-amir73il@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180830151551.27422-3-amir73il@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu 30-08-18 18:15:50, Amir Goldstein wrote: > Send events to group if super block mark mask matches the event > and unless the same group has an ignore mask on the vfsmount or > the inode on which the event occurred. > > Soon, fanotify backend is going to support super block marks and > fanotify backend only supports path type events. > > Signed-off-by: Amir Goldstein Two small questions below. Otherwise the patch looks good to me. > --- > fs/notify/fsnotify.c | 27 +++++++++++++++++---------- > 1 file changed, 17 insertions(+), 10 deletions(-) > ... > if (!(mask & FS_MODIFY) && > !(test_mask & to_tell->i_fsnotify_mask) && > - !(mnt && test_mask & mnt->mnt_fsnotify_mask)) > + !(mnt && (test_mask & mnt_or_sb_mask))) When you use mnt_or_sb_mask, the 'mnt' check is useless, right? > iter_info.srcu_idx = srcu_read_lock(&fsnotify_mark_srcu); > @@ -364,16 +367,20 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is, > } > > if (mnt && ((mask & FS_MODIFY) || > - (test_mask & mnt->mnt_fsnotify_mask))) { > + (test_mask & mnt_or_sb_mask))) { > iter_info.marks[FSNOTIFY_OBJ_TYPE_INODE] = > fsnotify_first_mark(&to_tell->i_fsnotify_marks); > iter_info.marks[FSNOTIFY_OBJ_TYPE_VFSMOUNT] = > fsnotify_first_mark(&mnt->mnt_fsnotify_marks); > + if ((mask & FS_MODIFY) || > + (test_mask & sb->s_fsnotify_mask)) Why is here this additional test? We might need to clear ignore masks on SB list if nothing else. Also we need to reflect ignore mask from the superblock marks... I agree there's probably no huge use for either of these two functionalities but I just don't see a strong reason for sb & mnt marks to behave differently. > + iter_info.marks[FSNOTIFY_OBJ_TYPE_SB] = > + fsnotify_first_mark(&sb->s_fsnotify_marks); > } > > /* > - * We need to merge inode & vfsmount mark lists so that inode mark > - * ignore masks are properly reflected for mount mark notifications. > + * We need to merge inode/vfsmount/sb mark lists so that e.g. inode mark > + * ignore masks are properly reflected for mount/sb mark notifications. > * That's why this traversal is so complicated... > */ > while (fsnotify_iter_select_report_types(&iter_info)) { > -- > 2.7.4 > Honza -- Jan Kara SUSE Labs, CR