From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:56342 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728172AbeHaSNb (ORCPT ); Fri, 31 Aug 2018 14:13:31 -0400 Date: Fri, 31 Aug 2018 16:05:51 +0200 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Marko Rauhamaa , linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org Subject: Re: [PATCH v3 3/3] fanotify: add API to attach/detach super block mark Message-ID: <20180831140551.GK11622@quack2.suse.cz> References: <20180830151551.27422-1-amir73il@gmail.com> <20180830151551.27422-4-amir73il@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180830151551.27422-4-amir73il@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu 30-08-18 18:15:51, Amir Goldstein wrote: > Add another mark type flag FAN_MARK_FILESYSTEM for add/remove/flush > of super block mark type. > > A super block watch gets all events on the filesystem, regardless of > the mount from which the mark was added, unless an ignore mask exists > on either the inode or the mount where the event was generated. > > Only one of FAN_MARK_MOUNT and FAN_MARK_FILESYSTEM mark type flags > may be provided to fanotify_mark() or no mark type flag for inode mark. > > Cc: > Signed-off-by: Amir Goldstein Just one nit below, otherwise the patch look good to me. > diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h > index 74247917de04..7345b9a57f66 100644 > --- a/include/uapi/linux/fanotify.h > +++ b/include/uapi/linux/fanotify.h > @@ -43,6 +43,7 @@ > FAN_UNLIMITED_MARKS) > > /* flags used for fanotify_modify_mark() */ > +#define FAN_MARK_INODE 0x00000000 > #define FAN_MARK_ADD 0x00000001 > #define FAN_MARK_REMOVE 0x00000002 > #define FAN_MARK_DONT_FOLLOW 0x00000004 > @@ -51,6 +52,7 @@ > #define FAN_MARK_IGNORED_MASK 0x00000020 > #define FAN_MARK_IGNORED_SURV_MODIFY 0x00000040 > #define FAN_MARK_FLUSH 0x00000080 > +#define FAN_MARK_FILESYSTEM 0x00000100 > > #define FAN_ALL_MARK_FLAGS (FAN_MARK_ADD |\ > FAN_MARK_REMOVE |\ > @@ -59,7 +61,10 @@ > FAN_MARK_MOUNT |\ > FAN_MARK_IGNORED_MASK |\ > FAN_MARK_IGNORED_SURV_MODIFY |\ > - FAN_MARK_FLUSH) > + FAN_MARK_FLUSH|\ > + FAN_MARK_FILESYSTEM) > + > +#define FAN_ALL_MARK_TYPE_FLAGS (FAN_MARK_MOUNT | FAN_MARK_FILESYSTEM) > > /* > * All of the events - we build the list by hand so that we can add flags in So for completeness I'd add FAN_MARK_INODE to FAN_ALL_MARK_FLAGS and FAN_ALL_MARK_TYPE_FLAGS. I know it doesn't change the actual value but logically it belongs there when you defined it... Also one more thing to consider: Different mark types cannot be combined. So it could save some bits in 'flags' in future if we had something like FAN_MARK_TYPE_MASK and (flags & FAN_MARK_TYPE_MASK) would enumerate different mark types - 0 for inode mark, FAN_MARK_MOUNT for mount mark, FAN_MARK_FILESYSTEM for superblock mark etc. Again, currently there's no practical difference in the values, just the names and tests for validity would be slightly different. Honza -- Jan Kara SUSE Labs, CR