Re: [PATCH v3 14/16] fanotify: implement "evictable" inode marks

[Amir Goldstein <amir73il@gmail.com>]

On Thu, Apr 21, 2022 at 6:40 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 13-04-22 12:09:33, Amir Goldstein wrote:
> > When an inode mark is created with flag FAN_MARK_EVICTABLE, it will not
> > pin the marked inode to inode cache, so when inode is evicted from cache
> > due to memory pressure, the mark will be lost.
> >
> > When an inode mark with flag FAN_MARK_EVICATBLE is updated without using
> > this flag, the marked inode is pinned to inode cache.
> >
> > When an inode mark is updated with flag FAN_MARK_EVICTABLE but an
> > existing mark already has the inode pinned, the mark update fails with
> > error EEXIST.
> >
> > Evictable inode marks can be used to setup inode marks with ignored mask
> > to suppress events from uninteresting files or directories in a lazy
> > manner, upon receiving the first event, without having to iterate all
> > the uninteresting files or directories before hand.
> >
> > The evictbale inode mark feature allows performing this lazy marks setup
> > without exhausting the system memory with pinned inodes.
> >
> > This change does not enable the feature yet.
> >
> > Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxiRDpuS=2uA6+ZUM7yG9vVU-u212tkunBmSnP_u=mkv=Q@mail.gmail.com/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Just one nit below...
>
> > @@ -1097,6 +1099,18 @@ static int fanotify_mark_update_flags(struct fsnotify_mark *fsn_mark,
> >                       *recalc = true;
> >       }
> >
> > +     if (fsn_mark->connector->type != FSNOTIFY_OBJ_TYPE_INODE ||
> > +         want_iref == !(fsn_mark->flags & FSNOTIFY_MARK_FLAG_NO_IREF))
> > +             return 0;
> > +
> > +     /*
> > +      * NO_IREF may be removed from a mark, but not added.
> > +      * When removed, fsnotify_recalc_mask() will take the inode ref.
> > +      */
> > +     WARN_ON_ONCE(!want_iref);
> > +     fsn_mark->flags &= ~FSNOTIFY_MARK_FLAG_NO_IREF;
> > +     *recalc = true;
> > +
> >       return 0;
> >  }
>
> Since we always return 0 from this function, we may as well just drop the
> 'recalc' argument and return whether mask recalc is needed?
>

I agree, but in this case, I rather also return recalc from
fanotify_mark_add_to_mask() and keep fsnotify_recalc_mask()
in fanotify_add_mark() and it is now.

Thanks,
Amir.