NetBSD Problem Report #56788
From gson@gson.org Sun Apr 10 11:32:58 2022
Return-Path: <gson@gson.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 60D641A921F
for <gnats-bugs@gnats.NetBSD.org>; Sun, 10 Apr 2022 11:32:58 +0000 (UTC)
Message-Id: <20220410113250.64ED02546F8@guava.gson.org>
Date: Sun, 10 Apr 2022 14:32:50 +0300 (EEST)
From: gson@gson.org (Andreas Gustafsson)
Reply-To: gson@gson.org (Andreas Gustafsson)
To: gnats-bugs@NetBSD.org
Subject: Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
X-Send-Pr-Version: 3.95
>Number: 56788
>Category: port-sparc
>Synopsis: Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: port-sparc-maintainer
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sun Apr 10 11:35:00 +0000 2022
>Last-Modified: Mon Jun 20 02:40:01 +0000 2022
>Originator: Andreas Gustafsson
>Release: NetBSD-current of 2011 to present
>Organization:
>Environment:
System: NetBSD
Architecture: sparc
Machine: sparc
>Description:
On the TNF sparc testbed, various networking tests randomly fail with
a rump kernel panic:
[ 1.3700050] panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed: file "/tmp/build/2022.04.05.05.04.04-sparc/src/sys/rump/net/lib/libshmif/if_shmem.c", line 157
Logs from the most recent occurrences:
http://releng.netbsd.org/b5reports/sparc/2022/2022.03.25.23.16.04/test.html#net_icmp_t_ping_ping_of_death
http://releng.netbsd.org/b5reports/sparc/2022/2022.03.29.22.29.29/test.html#net_icmp_t_ping_pingsize
http://releng.netbsd.org/b5reports/sparc/2022/2022.04.05.05.04.04/test.html#net_bpfilter_t_bpfilter_bpfiltermchain
This is a long-standing issue, going back at least to 2011. I have
not seen it on architectures other than sparc.
>How-To-Repeat:
>Fix:
>Audit-Trail:
From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@NetBSD.org
Cc: port-hppa-maintainer@netbsd.org
Subject: Re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
Date: Sun, 12 Jun 2022 14:19:35 -0400
Andreas Gustafsson writes:
> On the TNF sparc testbed, various networking tests randomly fail with
> a rump kernel panic:
> [ 1.3700050] panic: kernel diagnostic assertion "old =3D=3D LOCK_LOCK=
ED" failed: file "/tmp/build/2022.04.05.05.04.04-sparc/src/sys/rump/net/li=
b/libshmif/if_shmem.c", line 157
> This is a long-standing issue, going back at least to 2011. I have
> not seen it on architectures other than sparc.
I see this on HPPA as well, and I concur that it seems to be the
underlying cause of intermittent failures in various ATF net/ tests.
For example, if I run
$ while /usr/tests/net/icmp/t_ping floodping2
do
:
done
for awhile, many iterations produce such a report -- though t_ping
usually claims it succeeded anyway. Maybe it's atf-run's responsibility
to notice rump kernel failure?
The failing assertion is in shmif_unlockbus():
old =3D atomic_swap_32(&busmem->shm_lock, LOCK_UNLOCKED);
KASSERT(old =3D=3D LOCK_LOCKED);
which for me immediately raises the suspicion that something's busted
in either atomic_swap_32 or atomic_cas_32 (the function used earlier to
acquire the shm_lock). On HPPA, atomic_swap_32 is implemented atop
atomic_cas_32, so that reduces to just one target of suspicion. In
kernel space, atomic_cas_32 is supposed to be implemented by _lock_cas
in lock_stubs.S, and (on my uniprocessor machine) that is a non-atomic
instruction sequence that is supposed to be made to act atomic by the
RAS restart mechanism. I can think of a few ways this might be going
wrong:
1. Maybe the rump kernel is linked to the userspace version of
atomic_cas_32, so that the address range being defended by the RAS
mechanism is the wrong one.
2. Maybe the rump kernel fails to implement the RAS checks in the
traps it takes.
3. Maybe this is related to PR 56837, and there is some case where
we can trap at _lock_cas_ras_start with tf_iioq_tail different from
_lock_cas_ras_start + 4.
It looks like uniprocessor SPARC also relies on RAS to make =
atomic_cas_32 atomic, so Occam's razor suggests that the failure
mechanism is the same on both archs, which would favor explanation
1 or 2 over 3. However, then we'd expect to see these on every
arch that uses RAS for atomic_cas_32, which I think is more than
SPARC and HPPA.
I just managed to get a core dump out of t_ping with librumpnet_shmif.so
attached, and it kind of looks like theory #1 might be the winner:
(gdb) x/16i atomic_cas_32
0xad53e0c0 <_atomic_cas_32>: addil L%2800,r19,r1
0xad53e0c4 <_atomic_cas_32+4>: ldw 3a4(r1),ret0
0xad53e0c8 <_atomic_cas_32+8>: stw rp,-14(sp)
0xad53e0cc <_atomic_cas_32+12>: stw,ma r4,40(sp)
0xad53e0d0 <_atomic_cas_32+16>: stw r19,-20(sp)
0xad53e0d4 <_atomic_cas_32+20>: ldw 0(ret0),r22
0xad53e0d8 <_atomic_cas_32+24>: b,l 0xad5375e8,r31
0xad53e0dc <_atomic_cas_32+28>: copy r31,rp
0xad53e0e0 <_atomic_cas_32+32>: ldw -54(sp),rp
0xad53e0e4 <_atomic_cas_32+36>: bv r0(rp)
0xad53e0e8 <_atomic_cas_32+40>: ldw,mb -40(sp),r4
0xad53e0ec <_atomic_cas_16>: addil L%2800,r19,r1
0xad53e0f0 <_atomic_cas_16+4>: ldw 3a8(r1),ret0
0xad53e0f4 <_atomic_cas_16+8>: stw rp,-14(sp)
0xad53e0f8 <_atomic_cas_16+12>: extrw,u r24,31,16,r24
0xad53e0fc <_atomic_cas_16+16>: stw,ma r4,40(sp)
This is not the code from lock_stubs.S.
Maybe there is an arch-specific linking problem here? Or maybe I don't
understand how this stuff is supposed to work in a rump kernel.
But in any case my money is on RAS not doing what it needs to.
regards, tom lane
From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@netbsd.org
Cc: port-sparc-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, gson@gson.org (Andreas Gustafsson)
Subject: re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
Date: Mon, 13 Jun 2022 10:07:12 +1000
sys/arch/hppa/hppa/lock_stubs.S is kernel-only. looks like
the hppa one comes here:
libc.a:atomic_init_testset.o:0000013c T _atomic_cas_32
.mrg.
From: Nick Hudson <nick.hudson@gmx.co.uk>
To: matthew green <mrg@eterna.com.au>, gnats-bugs@netbsd.org
Cc: port-sparc-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, Andreas Gustafsson <gson@gson.org>
Subject: Re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion
"old == LOCK_LOCKED" failed
Date: Mon, 13 Jun 2022 12:55:59 +0100
On 13/06/2022 01:07, matthew green wrote:
> sys/arch/hppa/hppa/lock_stubs.S is kernel-only. looks like
> the hppa one comes here:
>
> libc.a:atomic_init_testset.o:0000013c T _atomic_cas_32
I think that some (more) magic needs to be added so that the RAS section
of rumpns__atomic_cas_32 needs the appropriate rasctl(2) call.
Nick
From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@NetBSD.org, port-hppa-maintainer@netbsd.org
Cc:
Subject: Re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
Date: Tue, 14 Jun 2022 21:18:21 -0400
> I think that some (more) magic needs to be added so that the RAS section
> of rumpns__atomic_cas_32 needs the appropriate rasctl(2) call.
Hmm. I've not found what bit of magic prepends "rumpns_" to the symbol
visible in the source code, but I wonder whether the RAS_ADDR() macro
talks to it. Maybe librump's copy of __libc_atomic_init is redundantly
re-registering the RAS address ranges in libc's atomic_init_testset.o,
rather than the ones in librump's copy?
regards, tom lane
From: Nick Hudson <nick.hudson@gmx.co.uk>
To: Tom Lane <tgl@sss.pgh.pa.us>, gnats-bugs@NetBSD.org,
port-hppa-maintainer@netbsd.org
Cc:
Subject: Re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion
"old == LOCK_LOCKED" failed
Date: Wed, 15 Jun 2022 07:22:40 +0100
On 15/06/2022 02:18, Tom Lane wrote:
>> I think that some (more) magic needs to be added so that the RAS sectio=
n
>> of rumpns__atomic_cas_32 needs the appropriate rasctl(2) call.
>
> Hmm. I've not found what bit of magic prepends "rumpns_" to the symbol
> visible in the source code, but I wonder whether the RAS_ADDR() macro
> talks to it. Maybe librump's copy of __libc_atomic_init is redundantly
> re-registering the RAS address ranges in libc's atomic_init_testset.o,
> rather than the ones in librump's copy?
who knows? I'm not a member of the magic circle
I think this is where the symbol renaming happens
https://nxr.netbsd.org/xref/src/sys/rump/Makefile.rump#255
Nick
From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@NetBSD.org
Cc: port-hppa-maintainer@netbsd.org
Subject: Re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
Date: Fri, 17 Jun 2022 17:46:19 -0400
I've spent a great deal of time digging into the t_ping test case,
and I think I see what is happening on HPPA ... but I'm not sure
if it explains anything for SPARC.
In the first place, my theory about bogus RAS setup is all wet.
What's actually being used in librump for HPPA is
sys/rump/librump/rumpkern/atomic_cas_generic.c
which does not use the RAS mechanism. Instead it relies on
__cpu_simple_lock() which is a simple LDCW spinlock, which looks
entirely bulletproof --- but by experiment, it fails intermittently
in various t_ping test cases. I eventually realized that each
of the problem tests is forking a child process and then running
rump kernels in both the parent and child processes. The two
kernels communicate via a memory-mapped host file (cf
rump_pub_shmif_create) and we're trying to synchronize via
atomic_cas_32() applied to a word in that shared memory. That's
fine, but what makes atomic_cas_32 actually atomic? AFAICS, the
LDCW spinlock storage is a static array in atomic_cas_generic.c,
which means that *each rump kernel has its own copy* after the
fork. Therefore, __cpu_simple_lock() successfully interlocks
between threads in each rump kernel, but not between threads in
the two kernels, making atomic_cas_32() not at all atomic for
if_shmem.c's "bus" lock.
According to Makefile.rumpkern, atomic_cas_generic.c is used on
.if (${MACHINE_CPU} == "arm" && "${FEAT_LDREX}" != "yes") \
|| ${MACHINE_ARCH} == "coldfire" || ${MACHINE_CPU} == "hppa" \
|| ${MACHINE_CPU} == "mips" || ${MACHINE_CPU} == "sh3" \
|| ${MACHINE_ARCH} == "vax" || ${MACHINE_ARCH} == "m68000"
so I'd kind of expect these tests to fail on all of those arches.
(Their spinlock mechanisms vary of course, but the problem of
the spinlock data being process-local will be the same for all.)
It's striking though that SPARC is not in this list. Perhaps
it has some related failure mechanism? I see that
src/common/lib/libc/arch/sparc/atomic/atomic_cas.S
makes use of a "locktab array" that looks like it'd have the
same problem of being process-local, but I am not sure if that
code is used in a rump kernel, or whether it applies to the
SPARC variant being complained of here.
Anyway, I'm not sure that I see a practical solution to make
these test cases work on these arches. Potentially we could
add a spinlock field to struct shmif_mem, but getting
atomic_cas_32 to use it would entail some ugly API changes.
Maybe it's better just to skip the problematic test cases
on these arches.
regards, tom lane
From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@NetBSD.org
Cc: port-hppa-maintainer@netbsd.org
Subject: Re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
Date: Fri, 17 Jun 2022 19:47:37 -0400
... BTW, I now believe that this same issue affects a number of
other test cases that have been unstable for me, including
net/if_gif/t_gif:gif_basic_ipv6overipv6
net/if_ipsec/t_ipsec:ipsecif_recursive_ipv6overipv4_esp_null
net/if_lagg/t_lagg:lagg_lacp_vlan_ipv6
net/if_lagg/t_lagg:lagg_lacp_vlanl2tp_ipv6
net/if_wg/t_basic:wg_basic_ipv6_over_ipv6
net/if_wg/t_basic:wg_payload_sizes_ipv4_over_ipv4
net/if_wg/t_basic:wg_payload_sizes_ipv6_over_ipv6
net/if_wg/t_tunnel:wg_tunnel_ipv6_over_ipv4
net/mpls/t_ldp_regen:ldp_regen
I see some of these also failing in the ARMv5 tests at
https://www.netbsd.org/~martin/evbarm-atf/
which is very likely the same issue given that old ARM is
one of the affected arches.
Maybe there's enough test cases here to justify a messy solution?
I'm wondering about arranging for atomic_cas_generic.c's spinlock
array to be kept in shared memory somehow.
regards, tom lane
From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@netbsd.org, tgl@sss.pgh.pa.us
Cc: port-sparc-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, gson@gson.org (Andreas Gustafsson)
Subject: re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
Date: Sat, 18 Jun 2022 13:33:35 +1000
nice work Tom.
> It's striking though that SPARC is not in this list. Perhaps
> it has some related failure mechanism? I see that
> src/common/lib/libc/arch/sparc/atomic/atomic_cas.S
> makes use of a "locktab array" that looks like it'd have the
> same problem of being process-local, but I am not sure if that
> code is used in a rump kernel, or whether it applies to the
> SPARC variant being complained of here.
yeah - sparc uses a similar mechanism last i looked at this,
and tried to figured out why rump_server and more would not
exit on sparc, and it always came down to mutexes not working
like you're described here, but i didn't think about shared
process spaces not being shared now.
we knew this was a problem with some code bases (eg, related
to shared futexes), but i did not know this was used by our
own code/tests.
.mrg.
From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@NetBSD.org
Cc: port-hppa-maintainer@netbsd.org
Subject: Re: port-sparc/56788 Rump kernel panic: kernel diagnostic assertion "old == LOCK_LOCKED" failed
Date: Sun, 19 Jun 2022 22:36:31 -0400
------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2324668.1655692526.1@sss.pgh.pa.us>
I wrote:
> I'm wondering about arranging for atomic_cas_generic.c's spinlock
> array to be kept in shared memory somehow.
Attached is a very dirty proof-of-concept patch that does the above.
The good news is that it seems to completely resolve the t_ping test
failures for me. The bad news is that (1) there's a lot of work
left to make this committable, and (2) for me, it doesn't seem to
fix any other tests besides t_ping. (2) suggests that the path of
least resistance is to just disable that test on affected arches.
OTOH, it seems likely that more tests will hit this in future,
so maybe it's worth doing the requisite amount of work for.
I don't have the ability to take this any further, but if someone
else wants to, what remains to be done includes:
* It's not clear to me how to handle the fact that
make_atomic_cas_globally_atomic() is only needed on some arches.
I'm sure there's a convention for that, I just don't know it.
* This probably violates a lot of other NetBSD style as well.
* I've not done anything about the SPARC situation. Probably
atomic_cas.S could be adapted to use a similar mechanism, but
maybe it'd be better to start using atomic_cas_generic.c
in the SPARC rump kernel?
regards, tom lane
------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="global-atomic-cas.patch";
charset="us-ascii"
Content-ID: <2324668.1655692526.2@sss.pgh.pa.us>
Content-Description: global-atomic-cas.patch
Content-Transfer-Encoding: quoted-printable
Index: lib/librumpuser/rumpuser_mem.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/lib/librumpuser/rumpuser_mem.c,v
retrieving revision 1.2
diff -u -r1.2 rumpuser_mem.c
--- lib/librumpuser/rumpuser_mem.c 24 Aug 2014 14:37:31 -0000 1.2
+++ lib/librumpuser/rumpuser_mem.c 20 Jun 2022 02:16:10 -0000
@@ -102,6 +102,44 @@
ET(rv);
}
=
+int
+rumpuser_filemmap(void *prefaddr, int fd, size_t size, int alignbit,
+ int exec, void **memp)
+{
+ void *mem;
+ int prot, rv;
+
+ if (ftruncate(fd, size) =3D=3D -1) {
+ rv =3D errno;
+ ET(rv);
+ }
+
+#ifndef MAP_ALIGNED
+#define MAP_ALIGNED(a) 0
+ if (alignbit)
+ fprintf(stderr, "rumpuser_filemmap: warning, requested "
+ "alignment not supported by hypervisor\n");
+#endif
+
+#if defined(__sun__) && !defined(MAP_FILE)
+#define MAP_FILE 0
+#endif
+
+ prot =3D PROT_READ|PROT_WRITE;
+ if (exec)
+ prot |=3D PROT_EXEC;
+ mem =3D mmap(prefaddr, size, prot,
+ MAP_FILE | MAP_SHARED | MAP_ALIGNED(alignbit), fd, 0);
+ if (mem =3D=3D MAP_FAILED) {
+ rv =3D errno;
+ } else {
+ *memp =3D mem;
+ rv =3D 0;
+ }
+
+ ET(rv);
+}
+
void
rumpuser_unmap(void *addr, size_t len)
{
Index: sys/rump/include/rump/rumpuser.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/rump/include/rump/rumpuser.h,v
retrieving revision 1.116
diff -u -r1.116 rumpuser.h
--- sys/rump/include/rump/rumpuser.h 22 Mar 2020 13:30:10 -0000 1.116
+++ sys/rump/include/rump/rumpuser.h 20 Jun 2022 02:16:12 -0000
@@ -74,6 +74,7 @@
int rumpuser_malloc(size_t, int, void **);
void rumpuser_free(void *, size_t);
int rumpuser_anonmmap(void *, size_t, int, int, void **);
+int rumpuser_filemmap(void *, int, size_t, int, int, void **);
void rumpuser_unmap(void *, size_t);
=
/*
Index: sys/rump/librump/rumpkern/atomic_cas_generic.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/rump/librump/rumpkern/atomic_cas_generic.c,v
retrieving revision 1.3
diff -u -r1.3 atomic_cas_generic.c
--- sys/rump/librump/rumpkern/atomic_cas_generic.c 14 Mar 2021 22:56:39 -0=
000 1.3
+++ sys/rump/librump/rumpkern/atomic_cas_generic.c 20 Jun 2022 02:16:12 -0=
000
@@ -33,6 +33,13 @@
* This is basically common/lib/libc/atomic/atomic_init_testset.c
* which always assumes MP and never uses RAS. This is used only on
* some archs. See Makefile.rumpkern for more information.
+ *
+ * We provide atomicity by using spinlocks; to avoid unnecessary contenti=
on,
+ * one of several spinlocks is chosen based on the address of the target =
word.
+ * Ordinarily, we only need to interlock between threads in a single proc=
ess,
+ * so that a plain static array of spinlocks is fine. But some tests req=
uire
+ * interlocking between threads in different processes. To support that,
+ * we can move the spinlock array into shared memory.
*/
=
#include "atomic_op_namespace.h"
@@ -40,12 +47,26 @@
#include <sys/types.h>
#include <sys/atomic.h>
#include <sys/lock.h>
+#include <sys/errno.h>
+
+#include <rump/rump.h>
+#include <rump/rumpuser.h>
+
+struct atomic_locks_shmem {
+ __cpu_simple_lock_t shared_atomic_locks[128];
+ int atomic_locks_magic;
+#define ATOMIC_LOCKS_MAGIC 0x58274727
+};
+
+static int make_one_spinlock_file(const char *path, =
+ __cpu_simple_lock_t **atomic_locks_ptr);
=
#define I2 __SIMPLELOCK_UNLOCKED, __SIMPLELOCK_UNLOCKED,
#define I16 I2 I2 I2 I2 I2 I2 I2 I2
#define I128 I16 I16 I16 I16 I16 I16 I16 I16
=
-static __cpu_simple_lock_t atomic_locks32[128] =3D { I128 };
+static __cpu_simple_lock_t static_atomic_locks32[128] =3D { I128 };
+static __cpu_simple_lock_t *atomic_locks32 =3D static_atomic_locks32;
=
uint32_t
_atomic_cas_32(volatile uint32_t *ptr, uint32_t old, uint32_t new)
@@ -53,6 +74,14 @@
__cpu_simple_lock_t *lock;
uint32_t ret;
=
+ /*
+ * Note that we use only the low-order 10 bits of the ptr to choose a
+ * spinlock. If the target word is in shared memory, it's possible that
+ * different processes would have it mapped at different addresses. But
+ * so long as all processes map the shared memory on a VM page boundary,
+ * and the page size is at least 1KB, we'll choose the same spinlock in
+ * all processes.
+ */
lock =3D &atomic_locks32[((uintptr_t)ptr >> 3) & 127];
__cpu_simple_lock(lock);
ret =3D *ptr;
@@ -70,7 +99,8 @@
__strong_alias(_atomic_cas_32_ni,_atomic_cas_32)
=
#ifdef _LP64
-static __cpu_simple_lock_t atomic_locks64[128] =3D { I128 };
+static __cpu_simple_lock_t static_atomic_locks64[128] =3D { I128 };
+static __cpu_simple_lock_t *atomic_locks64 =3D static_atomic_locks64;
=
uint64_t
_atomic_cas_64(volatile uint64_t *ptr, uint64_t old, uint64_t new)
@@ -78,7 +108,8 @@
__cpu_simple_lock_t *lock;
uint64_t ret;
=
- lock =3D &atomic_locks64[((uintptr_t)ptr >> 4) & 127];
+ /* See comment above about using only 10 bits of the ptr */
+ lock =3D &atomic_locks64[((uintptr_t)ptr >> 3) & 127];
__cpu_simple_lock(lock);
ret =3D *ptr;
if (__predict_true(ret =3D=3D old)) {
@@ -124,3 +155,106 @@
atomic_op_alias(atomic_cas_ptr_ni,_atomic_cas_32)
__strong_alias(_atomic_cas_ptr_ni,_atomic_cas_32)
#endif
+
+/*
+ * Move the atomic_cas_generic spinlocks into shared memory,
+ * so that atomicity is guaranteed across multiple processes.
+ * This is a no-op if it was already done in this process.
+ * Returns 0 if okay, else an errno code.
+ */
+extern int make_atomic_cas_globally_atomic(void); /* XXX where to put thi=
s? */
+
+int
+make_atomic_cas_globally_atomic(void)
+{
+ int error;
+
+ if (atomic_locks32 =3D=3D static_atomic_locks32) {
+ error =3D make_one_spinlock_file("netbsd-atomic-cas-32-spinlocks",
+ &atomic_locks32);
+ if (error)
+ return error;
+ }
+#ifdef _LP64
+ if (atomic_locks64 =3D=3D static_atomic_locks64) {
+ error =3D make_one_spinlock_file("netbsd-atomic-cas-64-spinlocks",
+ &atomic_locks64);
+ if (error)
+ return error;
+ }
+#endif
+ return 0;
+}
+
+static int
+make_one_spinlock_file(const char *path, =
+ __cpu_simple_lock_t **atomic_locks_ptr)
+{
+ bool created_it =3D false;
+ int memfd =3D -1; /* XXXgcc */
+ void *mem;
+ struct atomic_locks_shmem *almem;
+ int error;
+
+ /*
+ * Create or open the backing file, detecting if we created it.
+ *
+ * XXX there's no provision to remove it at shutdown. This doesn't
+ * matter in typical use under atf-run, and even when not doing that,
+ * it's not terribly harmful if the file stays around. One might
+ * need to manually remove it if a rump kernel crashes while holding
+ * one of the spinlocks, but that should be mighty rare.
+ */
+ while ((error =3D rumpuser_open(path,
+ RUMPUSER_OPEN_RDWR, &memfd)) !=3D 0)
+ {
+ if (error !=3D ENOENT)
+ return error;
+ error =3D rumpuser_open(path,
+ RUMPUSER_OPEN_RDWR | RUMPUSER_OPEN_CREATE | RUMPUSER_OPEN_EXCL,
+ &memfd);
+ if (error) {
+ if (error !=3D EEXIST)
+ return error;
+ continue; /* try again to open without creating */
+ }
+ created_it =3D true;
+ break;
+ }
+
+ /* Map it */
+ error =3D rumpuser_filemmap(NULL, memfd, sizeof(*almem), 0, 0, &mem);
+ if (error) {
+ rumpuser_close(memfd);
+ /* XXX unlink if created_it? */
+ return error;
+ }
+ almem =3D mem;
+
+ /*
+ * If we created it, initialize the shared spinlocks. Otherwise,
+ * wait if necessary for the creator to do so.
+ */
+ if (created_it) {
+ KASSERT(almem->atomic_locks_magic =3D=3D 0);
+ for (int i =3D 0; i < 128; i++)
+ __cpu_simple_lock_init(&almem->shared_atomic_locks[i]);
+ almem->atomic_locks_magic =3D ATOMIC_LOCKS_MAGIC;
+ } else {
+ while (almem->atomic_locks_magic !=3D ATOMIC_LOCKS_MAGIC)
+ (void) rumpuser_clock_sleep(RUMPUSER_CLOCK_RELWALL,
+ 0, 10000000);
+ }
+
+ /*
+ * Now replace the pointer to the static spinlock array with the
+ * address of the shared spinlocks array. This is a bit shaky
+ * because in principle some other threads in our own process could
+ * be contending for a lock, and this could transiently let them both
+ * think they got it. In practice this function should only be
+ * called during fairly early initialization, so the risk seems low.
+ */
+ *atomic_locks_ptr =3D almem->shared_atomic_locks;
+
+ return 0;
+}
Index: sys/rump/net/lib/libshmif/if_shmem.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/rump/net/lib/libshmif/if_shmem.c,v
retrieving revision 1.84
diff -u -r1.84 if_shmem.c
--- sys/rump/net/lib/libshmif/if_shmem.c 9 Apr 2022 23:45:02 -0000 1.84
+++ sys/rump/net/lib/libshmif/if_shmem.c 20 Jun 2022 02:16:12 -0000
@@ -56,6 +56,8 @@
=
#include "shmif_user.h"
=
+extern int make_atomic_cas_globally_atomic(void); /* XXX where to put thi=
s? */
+
static int shmif_clone(struct if_clone *, int);
static int shmif_unclone(struct ifnet *);
=
@@ -240,6 +242,10 @@
void *mem;
int error;
=
+ error =3D make_atomic_cas_globally_atomic();
+ if (error)
+ return error;
+
error =3D rumpcomp_shmif_mmap(memfd, BUSMEM_SIZE, &mem);
if (error)
return error;
------- =_aaaaaaaaaa0--
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home