The current code pattern used with MP-safe event filters is verbose. I would like to find a more concise form before making more event filters MP-safe.
Of struct filterops callbacks, f_modify and f_process essentially use f_event. By utilizing the f_event function pointer, a subsystem can have unified modify and process callbacks that are shared between the filter types. The diff below does the unification for pipes. The gist might become clearer by comparing the resulting code to r1.128 of sys_pipe.c The use of indirection might obfuscate the code a little bit. However, the reduction in code size should outweigh this. Indirect function calls involve retpolines on certain architectures and consequently take more time than direct calls. The patch does not change the big picture. Function pointers are used throughout the kernel. I wrote a test program where one process writes bytes through a pipe to another process that uses poll(2) and read(2). On a relatively recent Ryzen 5000 series processor, about 0.17% of syscall samples where spent in a retpoline handler, scattered in various parts of the kernel. None of these retpoline samples happened to involve filt_pipemodify() or filt_pipeprocess(). In sum, I think the patch is an improvement. OK? Index: kern/sys_pipe.c =================================================================== RCS file: src/sys/kern/sys_pipe.c,v retrieving revision 1.133 diff -u -p -r1.133 sys_pipe.c --- kern/sys_pipe.c 13 Dec 2021 14:56:55 -0000 1.133 +++ kern/sys_pipe.c 8 Feb 2022 15:47:27 -0000 @@ -78,25 +78,18 @@ static const struct fileops pipeops = { void filt_pipedetach(struct knote *kn); int filt_piperead(struct knote *kn, long hint); -int filt_pipereadmodify(struct kevent *kev, struct knote *kn); -int filt_pipereadprocess(struct knote *kn, struct kevent *kev); -int filt_piperead_common(struct knote *kn, struct pipe *rpipe); int filt_pipewrite(struct knote *kn, long hint); -int filt_pipewritemodify(struct kevent *kev, struct knote *kn); -int filt_pipewriteprocess(struct knote *kn, struct kevent *kev); -int filt_pipewrite_common(struct knote *kn, struct pipe *rpipe); int filt_pipeexcept(struct knote *kn, long hint); -int filt_pipeexceptmodify(struct kevent *kev, struct knote *kn); -int filt_pipeexceptprocess(struct knote *kn, struct kevent *kev); -int filt_pipeexcept_common(struct knote *kn, struct pipe *rpipe); +int filt_pipemodify(struct kevent *kev, struct knote *kn); +int filt_pipeprocess(struct knote *kn, struct kevent *kev); const struct filterops pipe_rfiltops = { .f_flags = FILTEROP_ISFD | FILTEROP_MPSAFE, .f_attach = NULL, .f_detach = filt_pipedetach, .f_event = filt_piperead, - .f_modify = filt_pipereadmodify, - .f_process = filt_pipereadprocess, + .f_modify = filt_pipemodify, + .f_process = filt_pipeprocess, }; const struct filterops pipe_wfiltops = { @@ -104,8 +97,8 @@ const struct filterops pipe_wfiltops = { .f_attach = NULL, .f_detach = filt_pipedetach, .f_event = filt_pipewrite, - .f_modify = filt_pipewritemodify, - .f_process = filt_pipewriteprocess, + .f_modify = filt_pipemodify, + .f_process = filt_pipeprocess, }; const struct filterops pipe_efiltops = { @@ -113,8 +106,8 @@ const struct filterops pipe_efiltops = { .f_attach = NULL, .f_detach = filt_pipedetach, .f_event = filt_pipeexcept, - .f_modify = filt_pipeexceptmodify, - .f_process = filt_pipeexceptprocess, + .f_modify = filt_pipemodify, + .f_process = filt_pipeprocess, }; /* @@ -954,9 +947,9 @@ filt_pipedetach(struct knote *kn) } int -filt_piperead_common(struct knote *kn, struct pipe *rpipe) +filt_piperead(struct knote *kn, long hint) { - struct pipe *wpipe; + struct pipe *rpipe = kn->kn_fp->f_data, *wpipe; rw_assert_wrlock(rpipe->pipe_lock); @@ -975,49 +968,9 @@ filt_piperead_common(struct knote *kn, s } int -filt_piperead(struct knote *kn, long hint) -{ - struct pipe *rpipe = kn->kn_fp->f_data; - - return (filt_piperead_common(kn, rpipe)); -} - -int -filt_pipereadmodify(struct kevent *kev, struct knote *kn) -{ - struct pipe *rpipe = kn->kn_fp->f_data; - int active; - - rw_enter_write(rpipe->pipe_lock); - knote_modify(kev, kn); - active = filt_piperead_common(kn, rpipe); - rw_exit_write(rpipe->pipe_lock); - - return (active); -} - -int -filt_pipereadprocess(struct knote *kn, struct kevent *kev) -{ - struct pipe *rpipe = kn->kn_fp->f_data; - int active; - - rw_enter_write(rpipe->pipe_lock); - if (kev != NULL && (kn->kn_flags & EV_ONESHOT)) - active = 1; - else - active = filt_piperead_common(kn, rpipe); - if (active) - knote_submit(kn, kev); - rw_exit_write(rpipe->pipe_lock); - - return (active); -} - -int -filt_pipewrite_common(struct knote *kn, struct pipe *rpipe) +filt_pipewrite(struct knote *kn, long hint) { - struct pipe *wpipe; + struct pipe *rpipe = kn->kn_fp->f_data, *wpipe; rw_assert_wrlock(rpipe->pipe_lock); @@ -1036,49 +989,9 @@ filt_pipewrite_common(struct knote *kn, } int -filt_pipewrite(struct knote *kn, long hint) -{ - struct pipe *rpipe = kn->kn_fp->f_data; - - return (filt_pipewrite_common(kn, rpipe)); -} - -int -filt_pipewritemodify(struct kevent *kev, struct knote *kn) -{ - struct pipe *rpipe = kn->kn_fp->f_data; - int active; - - rw_enter_write(rpipe->pipe_lock); - knote_modify(kev, kn); - active = filt_pipewrite_common(kn, rpipe); - rw_exit_write(rpipe->pipe_lock); - - return (active); -} - -int -filt_pipewriteprocess(struct knote *kn, struct kevent *kev) -{ - struct pipe *rpipe = kn->kn_fp->f_data; - int active; - - rw_enter_write(rpipe->pipe_lock); - if (kev != NULL && (kn->kn_flags & EV_ONESHOT)) - active = 1; - else - active = filt_pipewrite_common(kn, rpipe); - if (active) - knote_submit(kn, kev); - rw_exit_write(rpipe->pipe_lock); - - return (active); -} - -int -filt_pipeexcept_common(struct knote *kn, struct pipe *rpipe) +filt_pipeexcept(struct knote *kn, long hint) { - struct pipe *wpipe; + struct pipe *rpipe = kn->kn_fp->f_data, *wpipe; int active = 0; rw_assert_wrlock(rpipe->pipe_lock); @@ -1096,29 +1009,21 @@ filt_pipeexcept_common(struct knote *kn, } int -filt_pipeexcept(struct knote *kn, long hint) -{ - struct pipe *rpipe = kn->kn_fp->f_data; - - return (filt_pipeexcept_common(kn, rpipe)); -} - -int -filt_pipeexceptmodify(struct kevent *kev, struct knote *kn) +filt_pipemodify(struct kevent *kev, struct knote *kn) { struct pipe *rpipe = kn->kn_fp->f_data; int active; rw_enter_write(rpipe->pipe_lock); knote_modify(kev, kn); - active = filt_pipeexcept_common(kn, rpipe); + active = kn->kn_fop->f_event(kn, 0); rw_exit_write(rpipe->pipe_lock); return (active); } int -filt_pipeexceptprocess(struct knote *kn, struct kevent *kev) +filt_pipeprocess(struct knote *kn, struct kevent *kev) { struct pipe *rpipe = kn->kn_fp->f_data; int active; @@ -1127,7 +1032,7 @@ filt_pipeexceptprocess(struct knote *kn, if (kev != NULL && (kn->kn_flags & EV_ONESHOT)) active = 1; else - active = filt_pipeexcept_common(kn, rpipe); + active = kn->kn_fop->f_event(kn, 0); if (active) knote_submit(kn, kev); rw_exit_write(rpipe->pipe_lock);