On Thu, Nov 30, 2023 at 4:25 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Sat 18-11-23 20:30:17, Amir Goldstein wrote:
> > Some filesystems like fuse and nfs have zero or non-unique fsid.
> > We would like to avoid reporting ambiguous fsid in events, so we need
> > to avoid marking objects with same fsid and different sb.
> >
> > To make this easier to enforce, store the fsid in the marks of the group
> > instead of in the shared conenctor.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> Very nice! I like the result. Just a few nits below.
>
> > +static inline __kernel_fsid_t *fanotify_mark_fsid(struct fsnotify_mark *mark)
> > +{
> > + return &FANOTIFY_MARK(mark)->fsid;
> > +}
>
> I guess, there's no big win in using this helper compared to using
> FANOTIFY_MARK(mark)->fsid so I'd just drop this helper.
ok.
>
> > @@ -530,6 +528,7 @@ struct fsnotify_mark {
> > #define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x0100
> > #define FSNOTIFY_MARK_FLAG_NO_IREF 0x0200
> > #define FSNOTIFY_MARK_FLAG_HAS_IGNORE_FLAGS 0x0400
> > +#define FSNOTIFY_MARK_FLAG_HAS_FSID 0x0800
> > unsigned int flags; /* flags [mark->lock] */
> > };
>
> So this flag is in fact private to fanotify notification framework. Either
> we could just drop this flag and use
>
> FANOTIFY_MARK(mark)->fsid[0] != 0 || FANOTIFY_MARK(mark)->fsid[1] != 0
Cannot.
Zero fsid is now a valid fsid in an inode mark (e.g. fuse).
The next patch also adds the flag FSNOTIFY_MARK_FLAG_WEAK_FSID
>
> instead or we could at least add a comment that this flags is in fact
> private to fanotify?
There is already a comment, because all the flags above are fanotify flags:
/* fanotify mark flags */
#define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x0100
#define FSNOTIFY_MARK_FLAG_NO_IREF 0x0200
#define FSNOTIFY_MARK_FLAG_HAS_IGNORE_FLAGS 0x0400
Thanks,
Amir.
