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);
