On Tue 29-03-22 10:49:00, Amir Goldstein wrote:
> Handle FAN_MARK_IGNORED_SURV_MODIFY flag change in a helper that
> is called after updating the mark mask.
>
> Move recalc of object mask inside fanotify_mark_add_to_mask() which
> makes the code a bit simpler to follow.
>
> Add also helper to translate fsnotify mark flags to user visible
> fanotify mark flags.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.h | 10 ++++++++
> fs/notify/fanotify/fanotify_user.c | 39 +++++++++++++++++-------------
> fs/notify/fdinfo.c | 6 ++---
> 3 files changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index a3d5b751cac5..87142bc0131a 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -490,3 +490,13 @@ static inline unsigned int fanotify_event_hash_bucket(
> {
> return event->hash & FANOTIFY_HTABLE_MASK;
> }
> +
> +static inline unsigned int fanotify_mark_user_flags(struct fsnotify_mark *mark)
> +{
> + unsigned int mflags = 0;
> +
> + if (mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)
> + mflags |= FAN_MARK_IGNORED_SURV_MODIFY;
> +
> + return mflags;
> +}
This, together with fdinfo change should probably be a separate commit. I
don't see a good reason to mix these two changes...
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 0f0db1efa379..6e78ea12239c 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1081,42 +1081,50 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group,
> flags, umask);
> }
>
> -static void fanotify_mark_add_ignored_mask(struct fsnotify_mark *fsn_mark,
> - __u32 mask, unsigned int flags,
> - __u32 *removed)
> +static int fanotify_mark_update_flags(struct fsnotify_mark *fsn_mark,
> + unsigned int flags, bool *recalc)
> {
> - fsn_mark->ignored_mask |= mask;
> -
> /*
> * Setting FAN_MARK_IGNORED_SURV_MODIFY for the first time may lead to
> * the removal of the FS_MODIFY bit in calculated mask if it was set
> * because of an ignored mask that is now going to survive FS_MODIFY.
> */
> if ((flags & FAN_MARK_IGNORED_SURV_MODIFY) &&
> + (flags & FAN_MARK_IGNORED_MASK) &&
> !(fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)) {
> fsn_mark->flags |= FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY;
> if (!(fsn_mark->mask & FS_MODIFY))
> - *removed = FS_MODIFY;
> + *recalc = true;
> }
> +
> + return 0;
> }
>
> -static __u32 fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark,
> - __u32 mask, unsigned int flags,
> - __u32 *removed)
> +static int fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark,
> + __u32 mask, unsigned int flags)
> {
> - __u32 oldmask, newmask;
> + __u32 oldmask;
> + bool recalc = false;
> + int ret;
>
> spin_lock(&fsn_mark->lock);
> oldmask = fsnotify_calc_mask(fsn_mark);
> if (!(flags & FAN_MARK_IGNORED_MASK)) {
> fsn_mark->mask |= mask;
> } else {
> - fanotify_mark_add_ignored_mask(fsn_mark, mask, flags, removed);
> + fsn_mark->ignored_mask |= mask;
> }
> - newmask = fsnotify_calc_mask(fsn_mark);
> +
> + recalc = fsnotify_calc_mask(fsn_mark) & ~oldmask &
> + ~fsnotify_conn_mask(fsn_mark->connector);
oldmask should be a subset of fsnotify_conn_mask() so the above should be
equivalent to:
recalc = fsnotify_calc_mask(fsn_mark) & ~fsnotify_conn_mask(fsn_mark->connector)
shouldn't it?
Otherwise this looks like a nice cleanup!
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
