kqueue: Only allow one thread to register event to a kqueue
authorSepherosa Ziehau <sephe@dragonflybsd.org>
Fri, 4 Mar 2016 14:15:54 +0000 (22:15 +0800)
committerSepherosa Ziehau <sephe@dragonflybsd.org>
Fri, 4 Mar 2016 14:15:54 +0000 (22:15 +0800)
This prevents various races on the registration path, if it
ever blocks (kq token will be released):
- Kqueue hash table creation; the malloc(M_WAIT) could block.
- Install a new event.  Holding the token protect the klist could block.

Since kqueue _should_ rarely be shared between threads, allowing one
thread to do the event registration to a kqueue can work well enough.

sys/kern/kern_event.c
sys/sys/eventvar.h

index 3b9d4ae..5ecd57e 100644 (file)
@@ -952,6 +952,7 @@ kqueue_register(struct kqueue *kq, struct kevent *kev)
        struct filterops *fops;
        struct file *fp = NULL;
        struct knote *kn = NULL;
+       struct thread *td;
        int error = 0;
 
        if (kev->filter < 0) {
@@ -974,8 +975,27 @@ kqueue_register(struct kqueue *kq, struct kevent *kev)
                        return (EBADF);
        }
 
+       td = curthread;
        lwkt_getpooltoken(kq);
 
+       /*
+        * Make sure that only one thread can register event on this kqueue,
+        * so that we would not suffer any race, even if the registration
+        * blocked, i.e. kq token was released, and the kqueue was shared
+        * between threads (this should be rare though).
+        */
+       while (__predict_false(kq->kq_regtd != NULL && kq->kq_regtd != td)) {
+               kq->kq_state |= KQ_REGWAIT;
+               tsleep(&kq->kq_regtd, 0, "kqreg", 0);
+       }
+       if (__predict_false(kq->kq_regtd != NULL)) {
+               /* Recursive calling of kqueue_register() */
+               td = NULL;
+       } else {
+               /* Owner of the kq_regtd, i.e. td != NULL */
+               kq->kq_regtd = td;
+       }
+
        if (fp != NULL) {
                list = &fp->f_klist;
        } else if (kq->kq_knhashmask) {
@@ -1140,6 +1160,13 @@ again:
        /* kn may be invalid now */
 
 done:
+       if (td != NULL) { /* Owner of the kq_regtd */
+               kq->kq_regtd = NULL;
+               if (__predict_false(kq->kq_state & KQ_REGWAIT)) {
+                       kq->kq_state &= ~KQ_REGWAIT;
+                       wakeup(&kq->kq_regtd);
+               }
+       }
        lwkt_relpooltoken(kq);
        if (fp != NULL)
                fdrop(fp);
index 8d755cb..c74f31c 100644 (file)
@@ -64,11 +64,13 @@ struct kqueue {
        struct          filedesc *kq_fdp;
        int             kq_state;
        u_int           kq_sleep_cnt;
+       struct          thread *kq_regtd;
        u_long          kq_knhashmask;          /* size of knhash */
        struct          klist *kq_knhash;       /* hash table for knotes */
 };
 
 #define KQ_ASYNC       0x04
+#define KQ_REGWAIT     0x08
 
 #endif /* _KERNEL */
 #endif /* !_SYS_EVENTVAR_H_ */