On Fri, Nov 15, 2024 at 6:11 PM Song Liu <songliubraving@xxxxxxxx> wrote:
>
> Hi Amir,
>
> > On Nov 15, 2024, at 12:51 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> [...]
>
> >>
> >> +#ifdef CONFIG_FANOTIFY_FASTPATH
> >> + fp_hook = srcu_dereference(group->fanotify_data.fp_hook, &fsnotify_mark_srcu);
> >> + if (fp_hook) {
> >> + struct fanotify_fastpath_event fp_event = {
> >> + .mask = mask,
> >> + .data = data,
> >> + .data_type = data_type,
> >> + .dir = dir,
> >> + .file_name = file_name,
> >> + .fsid = &fsid,
> >> + .match_mask = match_mask,
> >> + };
> >> +
> >> + ret = fp_hook->ops->fp_handler(group, fp_hook, &fp_event);
> >> + if (ret == FAN_FP_RET_SKIP_EVENT) {
> >> + ret = 0;
> >> + goto finish;
> >> + }
> >> + }
> >> +#endif
> >> +
> >
> > To me it makes sense that the fastpath module could also return a negative
> > (deny) result for permission events.
>
> Yes, this should just work. And I actually plan to use it.
>
> > Is there a specific reason that you did not handle this or just didn't think
> > of this option?
>
> But I haven't tested permission events yet. At first glance, maybe we just
> need to change the above code a bit, as:
>
>
> >> f (ret == FAN_FP_RET_SKIP_EVENT) {
> >> + ret = 0;
> >> + goto finish;
> >> + }
>
> if (ret != FAN_FP_RET_SEND_TO_USERSPACE) {
> if (ret == FAN_FP_RET_SKIP_EVENT)
> ret = 0;
> goto finish;
> }
>
> Well, I guess we should change the value of FAN_FP_RET_SEND_TO_USERSPACE,
> so that this condition will look better.
>
> We may also consider reorder the code so that we do not call
> fsnotify_prepare_user_wait() when the fastpath handles the event.
>
> Does this look reasonable?
Yes.
Thanks,
Amir.
