This patch makes pipes use the pipe pair lock instead of the kernel lock
for pipe klist serialization.

In pipe_kqfilter(), the pipe pair lock is already held, so it is
appropriate to keep on using klist_insert_locked() there.
filt_pipedetach(), on the other hand, can be shortened by using
klist_remove(). klist_remove() acquires the pipe pair lock internally.

Note that the pipe pair lock has to be held when calling KNOTE() with
a pipe klist as the argument. pipeselwakeup() calls KNOTE() directly
as well as indirectly through selwakeup().

There is a quirk in how pipe knotes are attached. Read filter knotes
attach to the pipe end at hand, whereas write filter knotes attach to the
other end of the pipe. As a consequence, when a pipe end has already been
closed, the klist can still contain knotes. This is why the klist_free()
calls are done only when the whole pair is destroyed.

OK?

Index: kern/sys_pipe.c
===================================================================
RCS file: src/sys/kern/sys_pipe.c,v
retrieving revision 1.125
diff -u -p -r1.125 sys_pipe.c
--- kern/sys_pipe.c     25 Dec 2020 12:59:52 -0000      1.125
+++ kern/sys_pipe.c     25 Dec 2020 15:10:05 -0000
@@ -126,6 +126,7 @@ void        pipe_iounlock(struct pipe *);
 int    pipe_iosleep(struct pipe *, const char *);
 
 struct pipe_pair *pipe_pair_create(void);
+void   pipe_pair_destroy(struct pipe_pair *);
 
 /*
  * The pipe system call for the DTYPE_PIPE type of pipes
@@ -853,8 +854,7 @@ pipe_destroy(struct pipe *cpipe)
        rw_exit_write(cpipe->pipe_lock);
 
        if (ppipe == NULL)
-               pool_put(&pipe_pair_pool, cpipe->pipe_pair);
+               pipe_pair_destroy(cpipe->pipe_pair);
 }
 
 /*
@@ -913,9 +914,7 @@ filt_pipedetach(struct knote *kn)
 {
        struct pipe *cpipe = kn->kn_hook;
 
-       rw_enter_write(cpipe->pipe_lock);
-       klist_remove_locked(&cpipe->pipe_sel.si_note, kn);
-       rw_exit_write(cpipe->pipe_lock);
+       klist_remove(&cpipe->pipe_sel.si_note, kn);
 }
 
 int
@@ -997,6 +996,9 @@ pipe_pair_create(void)
        pp->pp_wpipe.pipe_lock = &pp->pp_lock;
        pp->pp_rpipe.pipe_lock = &pp->pp_lock;
 
+       klist_init_rwlock(&pp->pp_wpipe.pipe_sel.si_note, &pp->pp_lock);
+       klist_init_rwlock(&pp->pp_rpipe.pipe_sel.si_note, &pp->pp_lock);
+
        if (pipe_create(&pp->pp_wpipe) || pipe_create(&pp->pp_rpipe))
                goto err;
        return (pp);
@@ -1005,3 +1007,11 @@ err:
        pipe_destroy(&pp->pp_rpipe);
        return (NULL);
 }
+
+void
+pipe_pair_destroy(struct pipe_pair *pp)
+{
+       klist_free(&pp->pp_wpipe.pipe_sel.si_note);
+       klist_free(&pp->pp_rpipe.pipe_sel.si_note);
+       pool_put(&pipe_pair_pool, pp);
+}

Reply via email to