There is a discrepancy between the KNOTE() macro and its manual page
entry. KNOTE() calls knote() if the list argument non-NULL. The manual
page says that knote() is called if the list is non-empty.

The manual page's notion originates from NetBSD. It differs from
FreeBSD.

I think the call on non-empty is more useful than the call on non-NULL.
The current users of KNOTE() should always pass a valid list the macro.
The implied non-empty check can be seen as a tiny optimization to avoid
a function call (for this, klist_empty() has to be inlined, otherwise
the check is pointless).

acpi_record_event() is a somewhat unusal caller of KNOTE() because it
uses an explicit pointer to the knote list head. The list head is
allocated in acpi_attach_common() but only when built without
SMALL_KERNEL. At a glance, it looks that the pointer might be NULL in
some cases. However, the KNOTE() macro can be reached only if the acpi
device is open. If SMALL_KERNEL is defined, acpiopen() always fails.

(acpi_record_event() could be clarified by adding a NULL check before
KNOTE(). Another option is to include <sys/event.h> in acpivar.h
for struct klist and replace the list head allocation, but <sys/event.h>
would become visible more widely.)

There is also the alternative of removing KNOTE() and calling knote()
directly without any preceding checks of emptiness. This would simplify
the interface because there would be no need to choose between knote()
and KNOTE(). However, the following patch is conservative and keeps the
macro for now.

OK?
 
Index: kern/kern_event.c
===================================================================
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.178
diff -u -p -r1.178 kern_event.c
--- kern/kern_event.c   25 Dec 2021 11:04:58 -0000      1.178
+++ kern/kern_event.c   7 Feb 2022 15:18:54 -0000
@@ -2069,12 +2069,6 @@ klist_remove_locked(struct klist *klist,
        SLIST_REMOVE(&klist->kl_list, kn, knote, kn_selnext);
 }
 
-int
-klist_empty(struct klist *klist)
-{
-       return (SLIST_EMPTY(&klist->kl_list));
-}
-
 /*
  * Detach all knotes from klist. The knotes are rewired to indicate EOF.
  *
Index: net/bpf.c
===================================================================
RCS file: src/sys/net/bpf.c,v
retrieving revision 1.211
diff -u -p -r1.211 bpf.c
--- net/bpf.c   5 Feb 2022 04:08:27 -0000       1.211
+++ net/bpf.c   7 Feb 2022 15:18:54 -0000
@@ -580,16 +580,12 @@ out:
 void
 bpf_wakeup(struct bpf_d *d)
 {
-       struct klist *klist;
-
        MUTEX_ASSERT_LOCKED(&d->bd_mtx);
 
        if (d->bd_nreaders)
                wakeup(d);
 
-       klist = &d->bd_sel.si_note;
-       if (!klist_empty(klist))
-               knote(klist, 0);
+       KNOTE(&d->bd_sel.si_note, 0);
 
        /*
         * As long as pgsigio() and selwakeup() need to be protected
Index: sys/event.h
===================================================================
RCS file: src/sys/sys/event.h,v
retrieving revision 1.61
diff -u -p -r1.61 event.h
--- sys/event.h 11 Dec 2021 09:28:26 -0000      1.61
+++ sys/event.h 7 Feb 2022 15:18:54 -0000
@@ -154,7 +154,7 @@ struct klist {
 
 #define KNOTE(list, hint)      do { \
                                        struct klist *__list = (list); \
-                                       if (__list != NULL) \
+                                       if (!klist_empty(__list)) \
                                                knote(__list, hint); \
                                } while (0)
 
@@ -312,9 +312,14 @@ extern void        klist_insert(struct klist *,
 extern void    klist_insert_locked(struct klist *, struct knote *);
 extern void    klist_remove(struct klist *, struct knote *);
 extern void    klist_remove_locked(struct klist *, struct knote *);
-extern int     klist_empty(struct klist *);
 extern void    klist_invalidate(struct klist *);
 
+static inline int
+klist_empty(struct klist *klist)
+{
+       return (SLIST_EMPTY(&klist->kl_list));
+}
+
 #else  /* !_KERNEL */
 
 #include <sys/cdefs.h>

Reply via email to