Module Name: src
Committed By: riastradh
Date: Sun Dec 1 15:28:20 UTC 2019
Modified Files:
src/sys/sys: pslist.h
src/tests/include/sys: t_pslist.c
Log Message:
Adapt <sys/pslist.h> to use atomic_load/store_*.
Changes:
- membar_producer();
*p = v;
=>
atomic_store_release(p, v);
(Effectively like using membar_exit instead of membar_producer,
which is what we should have been doing all along so that stores by
the `reader' can't affect earlier loads by the writer, such as
KASSERT(p->refcnt == 0) in the writer and atomic_inc(&p->refcnt) in
the reader.)
- p = *pp;
if (p != NULL) membar_datadep_consumer();
=>
p = atomic_load_consume(pp);
(Only makes a difference on DEC Alpha. As long as lists generally
have at least one element, this is not likely to make a big
difference, and keeps the code simpler and clearer.)
No other functional change intended. While here, annotate each
synchronizing load and store with its counterpart in a comment.
To generate a diff of this commit:
cvs rdiff -u -r1.6 -r1.7 src/sys/sys/pslist.h
cvs rdiff -u -r1.1 -r1.2 src/tests/include/sys/t_pslist.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/sys/pslist.h
diff -u src/sys/sys/pslist.h:1.6 src/sys/sys/pslist.h:1.7
--- src/sys/sys/pslist.h:1.6 Fri Sep 20 13:38:00 2019
+++ src/sys/sys/pslist.h Sun Dec 1 15:28:19 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: pslist.h,v 1.6 2019/09/20 13:38:00 maxv Exp $ */
+/* $NetBSD: pslist.h,v 1.7 2019/12/01 15:28:19 riastradh Exp $ */
/*-
* Copyright (c) 2016 The NetBSD Foundation, Inc.
@@ -65,7 +65,7 @@ static __inline void
pslist_init(struct pslist_head *head)
{
- head->plh_first = NULL;
+ head->plh_first = NULL; /* not yet published, so no atomic */
}
static __inline void
@@ -94,7 +94,7 @@ pslist_entry_destroy(struct pslist_entry
* would think they were simply at the end of the list.
* Instead, cause readers to crash.
*/
- entry->ple_next = _PSLIST_POISON;
+ atomic_store_relaxed(&entry->ple_next, _PSLIST_POISON);
}
/*
@@ -119,11 +119,10 @@ pslist_writer_insert_head(struct pslist_
_PSLIST_ASSERT(new->ple_prevp == NULL);
new->ple_prevp = &head->plh_first;
- new->ple_next = head->plh_first;
+ new->ple_next = head->plh_first; /* not yet published, so no atomic */
if (head->plh_first != NULL)
head->plh_first->ple_prevp = &new->ple_next;
- membar_producer();
- head->plh_first = new;
+ atomic_store_release(&head->plh_first, new);
}
static __inline void
@@ -138,9 +137,14 @@ pslist_writer_insert_before(struct pslis
_PSLIST_ASSERT(new->ple_prevp == NULL);
new->ple_prevp = entry->ple_prevp;
- new->ple_next = entry;
- membar_producer();
- *entry->ple_prevp = new;
+ new->ple_next = entry; /* not yet published, so no atomic */
+
+ /*
+ * Pairs with atomic_load_consume in pslist_reader_first or
+ * pslist_reader_next.
+ */
+ atomic_store_release(entry->ple_prevp, new);
+
entry->ple_prevp = &new->ple_next;
}
@@ -156,11 +160,12 @@ pslist_writer_insert_after(struct pslist
_PSLIST_ASSERT(new->ple_prevp == NULL);
new->ple_prevp = &entry->ple_next;
- new->ple_next = entry->ple_next;
+ new->ple_next = entry->ple_next; /* not yet published, so no atomic */
if (new->ple_next != NULL)
new->ple_next->ple_prevp = &new->ple_next;
- membar_producer();
- entry->ple_next = new;
+
+ /* Pairs with atomic_load_consume in pslist_reader_next. */
+ atomic_store_release(&entry->ple_next, new);
}
static __inline void
@@ -173,7 +178,15 @@ pslist_writer_remove(struct pslist_entry
if (entry->ple_next != NULL)
entry->ple_next->ple_prevp = entry->ple_prevp;
- *entry->ple_prevp = entry->ple_next;
+
+ /*
+ * No need for atomic_store_release because there's no
+ * initialization that this must happen after -- the store
+ * transitions from a good state with the entry to a good state
+ * without the entry, both of which are valid for readers to
+ * witness.
+ */
+ atomic_store_relaxed(entry->ple_prevp, entry->ple_next);
entry->ple_prevp = NULL;
/*
@@ -221,29 +234,30 @@ _pslist_writer_next_container(const stru
/*
* Reader operations. Caller must block pserialize_perform or
* equivalent and be bound to a CPU. Only plh_first/ple_next may be
- * read, and after that, a membar_datadep_consumer must precede
- * dereferencing the resulting pointer.
+ * read, and only with consuming memory order so that data-dependent
+ * loads happen afterward.
*/
static __inline struct pslist_entry *
pslist_reader_first(const struct pslist_head *head)
{
- struct pslist_entry *first = head->plh_first;
-
- if (first != NULL)
- membar_datadep_consumer();
-
- return first;
+ /*
+ * Pairs with atomic_store_release in pslist_writer_insert_head
+ * or pslist_writer_insert_before.
+ */
+ return atomic_load_consume(&head->plh_first);
}
static __inline struct pslist_entry *
pslist_reader_next(const struct pslist_entry *entry)
{
- struct pslist_entry *next = entry->ple_next;
+ /*
+ * Pairs with atomic_store_release in
+ * pslist_writer_insert_before or pslist_writer_insert_after.
+ */
+ struct pslist_entry *next = atomic_load_consume(&entry->ple_next);
_PSLIST_ASSERT(next != _PSLIST_POISON);
- if (next != NULL)
- membar_datadep_consumer();
return next;
}
@@ -252,12 +266,10 @@ static __inline void *
_pslist_reader_first_container(const struct pslist_head *head,
const ptrdiff_t offset)
{
- struct pslist_entry *first = head->plh_first;
+ struct pslist_entry *first = pslist_reader_first(head);
if (first == NULL)
return NULL;
- membar_datadep_consumer();
-
return (char *)first - offset;
}
@@ -265,13 +277,10 @@ static __inline void *
_pslist_reader_next_container(const struct pslist_entry *entry,
const ptrdiff_t offset)
{
- struct pslist_entry *next = entry->ple_next;
+ struct pslist_entry *next = pslist_reader_next(entry);
- _PSLIST_ASSERT(next != _PSLIST_POISON);
if (next == NULL)
return NULL;
- membar_datadep_consumer();
-
return (char *)next - offset;
}
Index: src/tests/include/sys/t_pslist.c
diff -u src/tests/include/sys/t_pslist.c:1.1 src/tests/include/sys/t_pslist.c:1.2
--- src/tests/include/sys/t_pslist.c:1.1 Sat Apr 9 04:39:47 2016
+++ src/tests/include/sys/t_pslist.c Sun Dec 1 15:28:20 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: t_pslist.c,v 1.1 2016/04/09 04:39:47 riastradh Exp $ */
+/* $NetBSD: t_pslist.c,v 1.2 2019/12/01 15:28:20 riastradh Exp $ */
/*-
* Copyright (c) 2016 The NetBSD Foundation, Inc.
@@ -29,16 +29,23 @@
* POSSIBILITY OF SUCH DAMAGE.
*/
-#include <sys/pslist.h>
-
-#include <atf-c.h>
-
/*
* XXX This is a limited test to make sure the operations behave as
* described on a sequential machine. It does nothing to test the
- * pserialize-safety of any operations.
+ * pserialize-safety of any operations. The following definitions must
+ * be replaced in any parallel tests.
*/
+#define atomic_load_relaxed(p) (*(p))
+#define atomic_load_acquire(p) (*(p))
+#define atomic_load_consume(p) (*(p))
+#define atomic_store_relaxed(p,v) (*(p) = (v))
+#define atomic_store_release(p,v) (*(p) = (v))
+
+#include <sys/pslist.h>
+
+#include <atf-c.h>
+
ATF_TC(misc);
ATF_TC_HEAD(misc, tc)
{
