> +bool fsnotify_prepare_user_wait(struct fsnotify_mark *inode_mark,
> + struct fsnotify_mark *vfsmount_mark,
> + int *srcu_idx)
> +{
> + struct fsnotify_group *group;
> +
> + if (WARN_ON_ONCE(!inode_mark && !vfsmount_mark))
> + return false;
> +
> + if (inode_mark)
> + group = inode_mark->group;
> + else
> + group = vfsmount_mark->group;
> +
> + /*
> + * Since acquisition of mark reference is an atomic op as well, we can
> + * be sure this inc is seen before any effect of refcount increment.
> + */
> + atomic_inc(&group->user_waits);
> +
> + if (inode_mark) {
> + /* This can fail if mark is being removed */
> + if (!fsnotify_get_mark_safe(inode_mark))
> + goto out_wait;
> + }
> + if (vfsmount_mark) {
> + if (!fsnotify_get_mark_safe(vfsmount_mark))
> + goto out_inode;
> + }
> +
> + /*
> + * Now that both marks are pinned by refcount we can drop SRCU lock.
> + * Marks can still be removed from the list but because of refcount
> + * they cannot be destroyed and we can safely resume the list iteration
> + * once userspace returns.
> + */
Jan,
Forgive me for hijacking this review for yet another cleanup proposal.
When I first looked at this function I thought:
"<sigh> again with those inode_mark, vfsmount_mark args.. oh well"
but then I took another look and it suddenly seems quite simple to get rid of
all the places that get passed these 2 args and simplify all of them
and mostly simplify send_to_group().
The plan is:
1. Return 1 from handle_event() => send_to_group() if event was
"dropped" by group.
2. backends may return "dropped" for several reasons (e.g. non-dir
inode in dnotify),
but the only interesting case to return "dropped" is in fanotify
if (!fanotify_should_send_event()), because only fanotify supports
vfsmount_mark
3. in fsnotify(), if (inode_group == vfsmount_group), pass only vfsmount_mark
to send_to_group() and check for "dropped" event. if event was dropped,
set inode_group = NULL, so inode_mark is iterated again. if event
wasn't dropped,
there is no reason to call send_to_group() again with inode_mark.
This logic change incurs a behavior change because fanotify_should_send_event()
for some reason combines the inode and vfsmount mark ignore masks to a single
unified ignore mask, so an ignore bit in just one of them would today cause not
sending event to group.
However, from reading the man page, this seems like a bug rather then desired
behavior, because the ignore mask should be relative to the object in question
and it should be cleared when the object in question is modified and
in that sense
a mount is a completely different object than an inode, so their ignore masks
should be independent as well and not unified.
I will send out a POC patch for you to consider along with the rest of your
very neat cleanups.
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
