kevent: Hold the knlist mutex when invoking f_event(NOTE_FORK)

[markj]

Summary

In general f_event is supposed to be called with the knlist mutex held;
this is one case where it is not. I think it is mostly harmless, but we
might as well obey the formal locking protocol.

PR: 291005

[kib]

sys/kern/kern_event.c

637

Wht do we need the knote list locked for kn_fflags modifications?

[markj]

sys/kern/kern_event.c

637

Isn't kn->kn_event effectively serialized by the knote list lock? I started reading the case in the kqueue_scan() loop where we handle EV_CLEAR and I don't see how this update is serialized against those stores.

[kib]

sys/kern/kern_event.c

637

You mean, that practically all modifications of kn_kevent are happen to occur under the kn list lock?

Sometimes the lock used as kn list lock is rw (e.g. for vnodes), and updates to kn_kevent can occur while lock is taken in shared mode. E.g. for vnode reads.

This should not invalidate the change above, since kn_lock() should be always exclusive in such cases, but it is not obvious to me that the statement about kn_kevent serialization is true.

Anyway, please add a comment.

[markj]

sys/kern/kern_event.c

637

Sometimes the lock used as kn list lock is rw (e.g. for vnodes), and updates to kn_kevent can occur while lock is taken in shared mode. E.g. for vnode reads.

Hmm, but the f_event implementations for vnodes (filt_vfsread(), filt_vfswrite(), filt_vfsvnode()) all acquire the vnode interlock internally, so they serialize with each other, and vfs_knllock() acquires the exclusive vnode lock, so I think updates are still serialized. Or am I missing something?

[kib]

sys/kern/kern_event.c

637

Yes, interlock there helps, using the common part of exclusive vnode lock vs. shared vnode lock + interlock. What I tried to say is that kn list lock probably not should be considered as kn_kevent lock due to this detail, in general.

[markj]

• Add a comment.
• Acquire the kqueue lock earlier if we're activating the knote, otherwise we will pick up and drop the kqueue lock twice in a row.

[kib]

sys/kern/kern_event.c

632

[markj]

Fix the comment.