[src/trunk]: src/sys/kern pcq(9): Fix consume operation in pcq_peek/get.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/649eeac6ad6c
branches:  trunk
changeset: 373656:649eeac6ad6c
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu Feb 23 03:01:22 2023 +0000

description:
pcq(9): Fix consume operation in pcq_peek/get.

These use atomic_load_consume to match the atomic_store_release in
pcq_put for pcq->pcq_items[c].

Reading the snapshot of pcq->pcq_pc need not be ordered:

- The consumer side (pcq_peek/get) is serialized by the API contract
  (single-consumer, multi-producer), so no ordering is necessary.

- The producer side updates pcq->pcq_pc first; if the consumer side
  sees that before the producer side has stored pcq->pcq_items[c],
  there's no problem -- it's as if the consumer had happened just a
  moment earlier and the producer hadn't entered pcq_put yet.

However, it should be an atomic load, not a plain load.  So use
atomic_load_relaxed, if for no other reason than to pacify thread
sanitizers.

XXX pullup-9
XXX pullup-10

diffstat:

 sys/kern/subr_pcq.c |  19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diffs (71 lines):

diff -r f2210eaa7f72 -r 649eeac6ad6c sys/kern/subr_pcq.c
--- a/sys/kern/subr_pcq.c       Thu Feb 23 03:00:53 2023 +0000
+++ b/sys/kern/subr_pcq.c       Thu Feb 23 03:01:22 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: subr_pcq.c,v 1.14 2023/02/23 03:00:53 riastradh Exp $  */
+/*     $NetBSD: subr_pcq.c,v 1.15 2023/02/23 03:01:22 riastradh Exp $  */
 
 /*-
  * Copyright (c) 2009, 2019 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_pcq.c,v 1.14 2023/02/23 03:00:53 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_pcq.c,v 1.15 2023/02/23 03:01:22 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -100,7 +100,7 @@
        KASSERT(item != NULL);
 
        do {
-               v = pcq->pcq_pc;
+               v = atomic_load_relaxed(&pcq->pcq_pc);
                pcq_split(v, &op, &c);
                p = pcq_advance(pcq, op);
                if (p == c) {
@@ -132,14 +132,13 @@
 void *
 pcq_peek(pcq_t *pcq)
 {
-       const uint32_t v = pcq->pcq_pc;
+       const uint32_t v = atomic_load_relaxed(&pcq->pcq_pc);
        u_int p, c;
 
        pcq_split(v, &p, &c);
 
        /* See comment on race below in pcq_get(). */
-       return (p == c) ? NULL :
-           (membar_datadep_consumer(), pcq->pcq_items[c]);
+       return (p == c) ? NULL : atomic_load_consume(&pcq->pcq_items[c]);
 }
 
 /*
@@ -154,15 +153,13 @@
        u_int p, c;
        void *item;
 
-       v = pcq->pcq_pc;
+       v = atomic_load_relaxed(&pcq->pcq_pc);
        pcq_split(v, &p, &c);
        if (p == c) {
                /* Queue is empty: nothing to return. */
                return NULL;
        }
-       /* Make sure we read pcq->pcq_pc before pcq->pcq_items[c].  */
-       membar_datadep_consumer();
-       item = pcq->pcq_items[c];
+       item = atomic_load_consume(&pcq->pcq_items[c]);
        if (item == NULL) {
                /*
                 * Raced with sender: we rely on a notification (e.g. softint
@@ -185,7 +182,7 @@
        membar_producer();
 #endif
        while (__predict_false(atomic_cas_32(&pcq->pcq_pc, v, nv) != v)) {
-               v = pcq->pcq_pc;
+               v = atomic_load_relaxed(&pcq->pcq_pc);
                pcq_split(v, &p, &c);
                c = pcq_advance(pcq, c);
                nv = pcq_combine(p, c);