patch: atfork unlock
Hi. As first mentioned on misc[1], I've identified a deadlock in libc
when a process forks, the children are multi-threaded, and they set
one or more atfork callbacks. The diff below causes ATFORK_UNLOCK() to
release the lock even when the process isn't multi-threaded. This
avoids the deadlock. With this patch applied, the test case I have for
this issue succeeds and there are no new failures during a full 'make
regress'.
Threading is outside my area of expertise so I've no idea if the fix
proposed here is appropriate. I'm happy to take or test feedback.
The diff is below and a clean copy is here:
https://www.packetmischief.ca/files/patches/atfork_on_fork.diff.
.joel
[1] https://marc.info/?l=openbsd-misc&m=166926508819790&w=2
Index: lib/libc/include/thread_private.h
===
RCS file: /data/cvs-mirror/OpenBSD/src/lib/libc/include/thread_private.h,v
retrieving revision 1.36
diff -p -u -r1.36 thread_private.h
--- lib/libc/include/thread_private.h 6 Jan 2021 19:54:17 - 1.36
+++ lib/libc/include/thread_private.h 8 Dec 2022 04:28:45 -
@@ -228,7 +228,7 @@ __END_HIDDEN_DECLS
} while (0)
#define _ATFORK_UNLOCK() \
do { \
- if (__isthreaded) \
+ if (_thread_cb.tc_atfork_unlock != NULL) \
_thread_cb.tc_atfork_unlock(); \
} while (0)
Index: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
===
RCS file: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
diff -N regress/lib/libpthread/pthread_atfork_on_fork/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libpthread/pthread_atfork_on_fork/Makefile 7 Dec 2022
04:38:39 -
@@ -0,0 +1,9 @@
+# $OpenBSD$
+
+PROG= pthread_atfork_on_fork
+
+REGRESS_TARGETS= timeout
+timeout:
+ timeout 10s ./${PROG}
+
+.include
Index: regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
===
RCS file: regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
diff -N regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
7 Dec 2022 04:59:10 -
@@ -0,0 +1,94 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2022 Joel Knight
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * This test exercises atfork lock/unlock through multiple generations of
+ * forked child processes where each child also becomes multi-threaded.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+#define NUMBER_OF_GENERATIONS 4
+#define NUMBER_OF_TEST_THREADS 2
+
+#define SAY(...) do { \
+fprintf(stderr, "pid %5i ", getpid()); \
+fprintf(stderr, __VA_ARGS__); \
+} while (0)
+
+void
+prepare(void)
+{
+/* Do nothing */
+}
+
+void *
+thread(void *arg)
+{
+return (NULL);
+}
+
+int
+test(int fork_level)
+{
+pid_t proc_pid;
+size_t thread_index;
+pthread_t threads[NUMBER_OF_TEST_THREADS];
+
+proc_pid = fork();
+fork_level = fork_level - 1;
+
+if (proc_pid == 0) {
+SAY("generation %i\n", fork_level);
+pthread_atfork(prepare, NULL, NULL);
+
+for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
thread_index++) {
+pthread_create(&threads[thread_index], NULL, thread, NULL);
+}
+
+for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
thread_index++) {
+pthread_join(threads[thread_index], NULL);
+}
+
+if (fork_level > 0) {
+test(fork_level);
+}
+
+SAY("exiting\n");
+exit(0);
+}
+else {
+SAY("parent waiting on child %i\n", proc_pid);
+waitpid(proc_pid, 0, 0);
+}
+
+return (0);
+}
+
+int
+main(int argc, char *argv[])
+{
+test(NUMBER_OF_GENERATIONS);
+
+return (0);
+}
Re: pvbus: pass M_ZERO properly
Yes
YASUOKA Masahiko wrote:
> This is obvious. M_ZERO must be for 3rd argument.
>
> ok?
>
> Index: sys/dev/pv/pvbus.c
> ===
> RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 pvbus.c
> --- sys/dev/pv/pvbus.c25 Aug 2022 17:38:16 - 1.25
> +++ sys/dev/pv/pvbus.c8 Dec 2022 02:32:46 -
> @@ -408,7 +408,7 @@ pvbusgetstr(size_t srclen, const char *s
> else if (srclen > PAGE_SIZE)
> return (ENAMETOOLONG);
>
> - *dstp = dst = malloc(srclen + 1, M_TEMP|M_ZERO, M_WAITOK);
> + *dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
> if (src != NULL) {
> error = copyin(src, dst, srclen);
> dst[srclen] = '\0';
>
Re: pvbus: pass M_ZERO properly
ok asou@
From: YASUOKA Masahiko
Date: Thu, 08 Dec 2022 11:35:33 +0900 (JST)
> This is obvious. M_ZERO must be for 3rd argument.
>
> ok?
>
> Index: sys/dev/pv/pvbus.c
> ===
> RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 pvbus.c
> --- sys/dev/pv/pvbus.c25 Aug 2022 17:38:16 - 1.25
> +++ sys/dev/pv/pvbus.c8 Dec 2022 02:32:46 -
> @@ -408,7 +408,7 @@ pvbusgetstr(size_t srclen, const char *s
> else if (srclen > PAGE_SIZE)
> return (ENAMETOOLONG);
>
> - *dstp = dst = malloc(srclen + 1, M_TEMP|M_ZERO, M_WAITOK);
> + *dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
> if (src != NULL) {
> error = copyin(src, dst, srclen);
> dst[srclen] = '\0';
>
ksh, test: test -t file descriptor isn't optional
Hi tech@,
Both test.1 and ksh.1 (under the non-POSIX compatibility flag) state
that `test -t` will default to test whether fd 1 is a TTY if the
argument is omitted. This isn't the case, and both treat `-t` as the
equivalent of `test -n -t`, ie, test if `-t` is a non-empty string,
trivially true.
It's easy to see it in `test`: it exits right away in switch(argc).
`ksh` is a bit more difficult to follow: builtins eventually call c_test
in c_test.c. For `test -t`, c_test will be called with wp being "test",
"-t". It'll eventually call test_parse, with the test environment set
with
te.pos.wp = wp + 1;
te.wp_end = wp + argc;
For this particular case, argc is 2. Bearing that in mind, test_parse
will make a call stack of test_aexpr -> test_oexpr -> test_nexpr ->
test_primary. The first 2 ifs are skipped (asuming no TEF_ERROR is set)
and the next if, which would handle `test -t` gracefully, isn't entered:
one of the && operands, &te->pos.wp[1] < te->wp_end, is false.
Afterwards, `-t` is evaled as TO_SNTZE, ie if it's a non-empty string.
The following diff amends the manpages, removing the "file descriptor
is optional" parts, and removes the unused machinery of bin/ksh/c_test.c
to deal with an optional argument in `test -t`, together with a small
rewrite in test_eval TO_FILTT case, as it was a bit too difficult for me
to read: it seems that, for legacy reason (haven't looked at the code
history), opnd1 could be NULL for TO_FILTT, the only test with such
behaviour. My understanding is that ptest_getopnd avoids that by
returning "1". So, opnd1 can't be NULL. bi_getn writes to res, but took
me a while to understand that looking only at the conditional call of
bi_getn in the if condition. Finally, for some reason, is opnd1 managed
to be NULL, isatty(0) is called. I believe that the intention was to do
res = opnd1 ? isatty(res) : 0;
instead. I think the proposed rewrite is easier to follow.
Regress is happy, although nothing in there tests `test -t` or `test -t
n`. Existing scripts shouldn't break with this change: `test -t` will
keep being true, whether fd 1 is a TTY or not. The diff can be further
split on man changes and code changes if needed.
btw, the easy test for `test -t` being wrong, whether under POSIX compat
or not, is
$ test -t 1; echo $? # 1 is TTY in an interactive session
0
$ test -t 1 >&-; echo $? # 1 isn't a TTY because it's closed
1
$ test -t >&-; echo $?
0
bye,
Lucas
diff 45d281fcfba6e40007d9a498265cdbf711d94ed0
ebffbda379cd24f3bb8c9e43b712b9d699c9980c
commit - 45d281fcfba6e40007d9a498265cdbf711d94ed0
commit + ebffbda379cd24f3bb8c9e43b712b9d699c9980c
blob - 7038a52bfa432d515d9683187930407c4d6bd6d5
blob + db16b71a9b64afb7047803c65ec620f03e18e42d
--- bin/ksh/c_test.c
+++ bin/ksh/c_test.c
@@ -156,12 +156,6 @@ c_test(char **wp)
}
if (argc == 1) {
opnd1 = (*te.getopnd)(&te, TO_NONOP, 1);
- /* Historically, -t by itself test if fd 1
-* is a file descriptor, but POSIX says its
-* a string test...
-*/
- if (!Flag(FPOSIX) && strcmp(opnd1, "-t") == 0)
- break;
res = (*te.eval)(&te, TO_STNZE, opnd1,
NULL, 1);
if (invert & 1)
@@ -271,14 +265,18 @@ test_eval(Test_env *te, Test_op op, const char *opnd1,
case TO_FILGZ: /* -s */
return stat(opnd1, &b1) == 0 && b1.st_size > 0L;
case TO_FILTT: /* -t */
- if (opnd1 && !bi_getn(opnd1, &res)) {
- te->flags |= TEF_ERROR;
- res = 0;
- } else {
- /* generate error if in FPOSIX mode? */
- res = isatty(opnd1 ? res : 0);
+ {
+ int s, v;
+
+ s = bi_getn(opnd1, &v);
+ if (!s) {
+ te->flags |= TEF_ERROR;
+ res = 0;
+ } else {
+ res = isatty(v);
+ }
+ return res;
}
- return res;
case TO_FILUID: /* -O */
return stat(opnd1, &b1) == 0 && b1.st_uid == ksheuid;
case TO_FILGID: /* -G */
@@ -527,7 +525,7 @@ ptest_getopnd(Test_env *te, Test_op op, int do_eval)
ptest_getopnd(Test_env *te, Test_op op, int do_eval)
{
if (te->pos.wp >= te->wp_end)
- return op == TO_FILTT ? "1" : NULL;
+ return NULL;
return *te->pos.wp++;
}
blob - 0e97b9b49a65d19722f1762d4cc0f36ffa6fae72
blob + 8d1542590b44b8b2aff63d44abe575d6845ed549
--- bin/ksh/ksh.1
+++ bin/ksh/ksh.1
@@ -2569
Re: Configure interface by lladdr during install
On Wed, Dec 07, 2022 at 09:51:51AM -0800, Andrew Hewus Fresh wrote:
> On Wed, Dec 07, 2022 at 10:28:05AM +, Stuart Henderson wrote:
> > On 2022/12/06 19:57, Andrew Hewus Fresh wrote:
> > > Which interface do you wish to configure? (name, lladdr, '?', or 'done')
> > > [vio0] ?
> > > Available network interfaces are: vio0 vlan0.
> > > vio0: lladdr fe:e1:bb:d1:dd:97
> > > Which interface do you wish to configure? (name, lladdr, '?', or 'done')
> > > [vio0] fe:e1:bb:d1:dd:97
> > > IPv4 address for vio0? (or 'autoconf' or 'none') [autoconf]
> > > IPv6 address for vio0? (or 'autoconf' or 'none') [none]
> > > Available network interfaces are: vio0 vlan0.
> >
> > It doesn't seem right to offer vio0 in the list if it was already
> > configured via lladdr?
>
> The installer currently allows re-configuring interfaces and deletes the
> hostname.if before doing it. I think the most useful solution is to
> check for "the other" configuration for that interface and remove it as
> well when reconfiguring.
Here's the next version that cleans up hostname.lladdr when configuring
by name and hostname.name when configuring by lladdr.
Available network interfaces are: vio0 vio1 vio2 vio3 vlan0.
Network interface to configure? (name, lladdr, '?', or 'done') [vio0]
IPv4 address for vio0? (or 'autoconf' or 'none') [autoconf] none
IPv6 address for vio0? (or 'autoconf' or 'none') [none] autoconf
Available network interfaces are: vio0 vio1 vio2 vio3 vlan0.
Network interface to configure? (name, lladdr, '?', or 'done') [done] ?
Available network interfaces are: vio0 vio1 vio2 vio3 vlan0.
vio0: lladdr fe:e1:bb:d1:dd:62
vio1: lladdr fe:e1:bb:d2:d2:99
vio2: lladdr fe:e1:bb:d3:2c:eb
vio3: lladdr fe:e1:bb:d4:08:54
Network interface to configure? (name, lladdr, '?', or 'done') [done]
fe:e1:bb:d1:dd:a2
'fe:e1:bb:d1:dd:a2' is not a valid choice.
Available network interfaces are: vio0 vio1 vio2 vio3 vlan0.
Network interface to configure? (name, lladdr, '?', or 'done') [done]
fe:e1:bb:d1:dd:62
IPv4 address for vio0? (or 'autoconf' or 'none') [autoconf] none
IPv6 address for vio0? (or 'autoconf' or 'none') [autoconf]
Available network interfaces are: vio0 vio1 vio2 vio3 vlan0.
Network interface to configure? (name, lladdr, '?', or 'done') [done] vio1
Symbolic (host) name for vio1? [foo]
IPv4 address for vio1? (or 'autoconf' or 'none') [autoconf] none
IPv6 address for vio1? (or 'autoconf' or 'none') [none] autoconf
Available network interfaces are: vio0 vio1 vio2 vio3 vlan0.
Network interface to configure? (name, lladdr, '?', or 'done') [done] !
Type 'exit' to return to install.
foo# grep . /tmp/i/hostname.*
/tmp/i/hostname.fe:e1:bb:d1:dd:62:inet6 autoconf
/tmp/i/hostname.vio1:inet6 autoconf
foo# exit
Network interface to configure? (name, lladdr, '?', or 'done') [done] vio0
IPv4 address for vio0? (or 'autoconf' or 'none') [autoconf] none
IPv6 address for vio0? (or 'autoconf' or 'none') [autoconf]
Available network interfaces are: vio0 vio1 vio2 vio3 vlan0.
Network interface to configure? (name, lladdr, '?', or 'done') [done] !
Type 'exit' to return to install.
foo# grep . /tmp/i/hostname.*
/tmp/i/hostname.vio0:inet6 autoconf
/tmp/i/hostname.vio1:inet6 autoconf
Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1215
diff -u -p -r1.1215 install.sub
--- distrib/miniroot/install.sub5 Dec 2022 20:12:00 - 1.1215
+++ distrib/miniroot/install.sub8 Dec 2022 03:22:07 -
@@ -356,6 +356,24 @@ get_ifs() {
done
}
+# Maps an interface name to lladdr,
+# filtered by whether it's valid for use by ifconfig -M
+if_name_to_lladdr() {
+ local _lladdr
+ _lladdr=$(ifconfig $1 | sed -n 's/^[[:space:]]*lladdr[[:space:]]//p')
+ [[ -n $_lladdr && -n $(ifconfig -M $_lladdr) ]] && echo $_lladdr
+}
+
+# Prints a list of interface names and lladdr from if_name_to_lladdr
+# to allow validating how we're allowed to configure interfaces.
+get_ifs_and_lladdrs() {
+ local _if
+ for _if in $(get_ifs); do
+ echo $_if
+ if_name_to_lladdr $_if
+ done
+}
+
# Return the device name of the disk device $1, which may be a disklabel UID.
get_dkdev_name() {
local _dev=${1#/dev/} _d
@@ -1291,7 +1309,8 @@ ieee80211_config() {
# Set up IPv4 and IPv6 interface configuration.
configure_ifs() {
- local _first _hn _if _name _p _vi _vn
+ local _first _hn _if _name _p _q _vi _vn
+ resp=
# Always need lo0 configured.
ifconfig lo0 inet 127.0.0.1/8
@@ -1306,14 +1325,48 @@ configure_ifs() {
[[ -n $_vi ]] && ((_vi++))
[[ -n $(get_ifs) ]] && _vn="vlan${_vi:-0}"
- ask_which "network interface" "do you wish to configure" \
- "\$(get_ifs) $_vn" \
- ${_p:-'$( (get_ifs netboot; get_ifs) | sed q )'}
- [[
Re: pvbus: pass M_ZERO properly
ok mvs@
> On 8 Dec 2022, at 05:35, YASUOKA Masahiko wrote:
>
> This is obvious. M_ZERO must be for 3rd argument.
>
> ok?
>
> Index: sys/dev/pv/pvbus.c
> ===
> RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 pvbus.c
> --- sys/dev/pv/pvbus.c25 Aug 2022 17:38:16 - 1.25
> +++ sys/dev/pv/pvbus.c8 Dec 2022 02:32:46 -
> @@ -408,7 +408,7 @@ pvbusgetstr(size_t srclen, const char *s
> else if (srclen > PAGE_SIZE)
> return (ENAMETOOLONG);
>
> - *dstp = dst = malloc(srclen + 1, M_TEMP|M_ZERO, M_WAITOK);
> + *dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
> if (src != NULL) {
> error = copyin(src, dst, srclen);
> dst[srclen] = '\0';
>
Re: pvbus: pass M_ZERO properly
On Thu, Dec 08, 2022 at 11:35:33AM +0900, YASUOKA Masahiko wrote:
> This is obvious. M_ZERO must be for 3rd argument.
>
> ok?
>
> Index: sys/dev/pv/pvbus.c
> ===
> RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 pvbus.c
> --- sys/dev/pv/pvbus.c25 Aug 2022 17:38:16 - 1.25
> +++ sys/dev/pv/pvbus.c8 Dec 2022 02:32:46 -
> @@ -408,7 +408,7 @@ pvbusgetstr(size_t srclen, const char *s
> else if (srclen > PAGE_SIZE)
> return (ENAMETOOLONG);
>
> - *dstp = dst = malloc(srclen + 1, M_TEMP|M_ZERO, M_WAITOK);
> + *dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
> if (src != NULL) {
> error = copyin(src, dst, srclen);
> dst[srclen] = '\0';
>
ok mlarkin. thanks!
pvbus: pass M_ZERO properly
This is obvious. M_ZERO must be for 3rd argument.
ok?
Index: sys/dev/pv/pvbus.c
===
RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
retrieving revision 1.25
diff -u -p -r1.25 pvbus.c
--- sys/dev/pv/pvbus.c 25 Aug 2022 17:38:16 - 1.25
+++ sys/dev/pv/pvbus.c 8 Dec 2022 02:32:46 -
@@ -408,7 +408,7 @@ pvbusgetstr(size_t srclen, const char *s
else if (srclen > PAGE_SIZE)
return (ENAMETOOLONG);
- *dstp = dst = malloc(srclen + 1, M_TEMP|M_ZERO, M_WAITOK);
+ *dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
if (src != NULL) {
error = copyin(src, dst, srclen);
dst[srclen] = '\0';
Re: Get rid of UVM_VNODE_CANPERSIST
> Date: Wed, 7 Dec 2022 16:51:49 +0100 > From: Theo Buehler > > On Wed, Dec 07, 2022 at 03:56:56PM +0100, Alexander Bluhm wrote: > > On Mon, Nov 28, 2022 at 03:04:17PM +0100, Mark Kettenis wrote: > > > So here is an updated diff that checks the UVM_VNODE_DYING flag and > > > skips the refcount manipulation if it is set. > > > > My macppc has build a full release with it. So it seems to fix the > > issue. I will continue testing. > > I have run a few days of snapshot builds in a loop on my m1 mini and had > this diff in my tree since it was posted on the list. The machine has > been stable as usual. Earlier attempts to solve this problem would have > blown up under this treatment. Thanks. I'll need to add a comment that explains what's going on, but will move forward with committing this in the next couple of days.
agtimer(4/armv7): switch to clockintr
ARMv7 has four interrupt clocks available. I think it'll be easier to
review/test if we do the clockintr switch driver by driver instead of
all at once in a massive single email. When all four driver patches
are confirmed to work, I'll commit them.
Here's a patch to switch agtimer(4/armv7) to clockintr.
- Remove agtimer-specific clock interrupt scheduling bits
and randomized statclock bits.
- Wire up agtimer_intrclock.
I am looking for a tester to help me get it compiling, and then run it
through a kernel-release-upgrade cycle.
Index: sys/arch/arm/include/cpu.h
===
RCS file: /cvs/src/sys/arch/arm/include/cpu.h,v
retrieving revision 1.61
diff -u -p -r1.61 cpu.h
--- sys/arch/arm/include/cpu.h 6 Jul 2021 09:34:06 - 1.61
+++ sys/arch/arm/include/cpu.h 7 Dec 2022 21:44:08 -
@@ -149,6 +149,7 @@ voidarm32_vector_init(vaddr_t, int);
* Per-CPU information. For now we assume one CPU.
*/
+#include
#include
#include
#include
@@ -198,7 +199,7 @@ struct cpu_info {
#ifdef GPROF
struct gmonparam *ci_gmon;
#endif
-
+ struct clockintr_queue ci_queue;
charci_panicbuf[512];
};
Index: sys/arch/arm/include/_types.h
===
RCS file: /cvs/src/sys/arch/arm/include/_types.h,v
retrieving revision 1.19
diff -u -p -r1.19 _types.h
--- sys/arch/arm/include/_types.h 5 Mar 2018 01:15:25 - 1.19
+++ sys/arch/arm/include/_types.h 7 Dec 2022 21:44:08 -
@@ -35,6 +35,8 @@
#ifndef _ARM__TYPES_H_
#define _ARM__TYPES_H_
+#define__HAVE_CLOCKINTR
+
#if defined(_KERNEL)
typedef struct label_t {
long val[11];
Index: sys/arch/arm/cortex/agtimer.c
===
RCS file: /cvs/src/sys/arch/arm/cortex/agtimer.c,v
retrieving revision 1.15
diff -u -p -r1.15 agtimer.c
--- sys/arch/arm/cortex/agtimer.c 12 Mar 2022 14:40:41 - 1.15
+++ sys/arch/arm/cortex/agtimer.c 7 Dec 2022 21:44:08 -
@@ -17,9 +17,11 @@
*/
#include
+#include
#include
#include
#include
+#include
#include
#include
@@ -51,28 +53,12 @@ static struct timecounter agtimer_timeco
.tc_priv = NULL,
};
-struct agtimer_pcpu_softc {
- uint64_tpc_nexttickevent;
- uint64_tpc_nextstatevent;
- u_int32_t pc_ticks_err_sum;
-};
-
struct agtimer_softc {
struct device sc_dev;
int sc_node;
-
- struct agtimer_pcpu_softc sc_pstat[MAXCPUS];
-
- u_int32_t sc_ticks_err_cnt;
u_int32_t sc_ticks_per_second;
- u_int32_t sc_ticks_per_intr;
- u_int32_t sc_statvar;
- u_int32_t sc_statmin;
-
-#ifdef AMPTIMER_DEBUG
- struct evcount sc_clk_count;
- struct evcount sc_stat_count;
-#endif
+ uint64_tsc_nsec_cycle_ratio;
+ uint64_tsc_nsec_max;
};
intagtimer_match(struct device *, void *, void *);
@@ -81,9 +67,11 @@ uint64_t agtimer_readcnt64(void);
intagtimer_intr(void *);
void agtimer_cpu_initclocks(void);
void agtimer_delay(u_int);
+void agtimer_rearm(void *, uint64_t);
void agtimer_setstatclockrate(int stathz);
void agtimer_set_clockrate(int32_t new_frequency);
void agtimer_startclock(void);
+void agtimer_trigger(void *, uint64_t);
const struct cfattach agtimer_ca = {
sizeof (struct agtimer_softc), agtimer_match, agtimer_attach
@@ -93,6 +81,11 @@ struct cfdriver agtimer_cd = {
NULL, "agtimer", DV_DULL
};
+struct intrclock agtimer_intrclock = {
+ .ic_rearm = agtimer_rearm,
+ .ic_trigger = agtimer_trigger
+};
+
uint64_t
agtimer_readcnt64(void)
{
@@ -155,16 +148,13 @@ agtimer_attach(struct device *parent, st
agtimer_frequency =
OF_getpropint(sc->sc_node, "clock-frequency", agtimer_frequency);
sc->sc_ticks_per_second = agtimer_frequency;
-
+ sc->sc_nsec_cycle_ratio =
+ sc->sc_ticks_per_second * (1ULL << 32) / 10;
+ sc->sc_nsec_max = UINT64_MAX / sc->sc_nsec_cycle_ratio;
printf(": %d kHz\n", sc->sc_ticks_per_second / 1000);
/* XXX: disable user access */
-#ifdef AMPTIMER_DEBUG
- evcount_attach(&sc->sc_clk_count, "clock", NULL);
- evcount_attach(&sc->sc_stat_count, "stat", NULL);
-#endif
-
/*
* private timer and interrupts not enabled until
* timer configures
@@ -175,8 +165,9 @@ agtimer_attach(struct device *parent, st
agtimer_timecounter.tc_frequency = sc->sc_ticks_per_second;
agtimer_timecounter.tc_priv = sc;
-
tc_init(&agtimer_timecounter);
+
+ agtimer_intrclock.ic_cookie = sc;
}
u_int
Re: Document 'uidinfo' structure locks
> On 7 Dec 2022, at 18:10, Alexander Bluhm wrote: > > On Thu, Dec 01, 2022 at 09:50:47PM +0300, Vitaliy Makkoveev wrote: > > You could also document these globals with [I] in kern_proc.c > > LIST_HEAD(uihashhead, uidinfo) *uihashtbl; > u_long uihash; /* size of hash table - 1 */ > > OK bluhm@ Indeed, except `uihashtbl’ is also [U].
Re: Configure interface by lladdr during install
On Wed, Dec 07, 2022 at 10:28:05AM +, Stuart Henderson wrote: > On 2022/12/06 19:57, Andrew Hewus Fresh wrote: > > Which interface do you wish to configure? (name, lladdr, '?', or 'done') > > [vio0] ? > > Available network interfaces are: vio0 vlan0. > > vio0: lladdr fe:e1:bb:d1:dd:97 > > Which interface do you wish to configure? (name, lladdr, '?', or 'done') > > [vio0] fe:e1:bb:d1:dd:97 > > IPv4 address for vio0? (or 'autoconf' or 'none') [autoconf] > > IPv6 address for vio0? (or 'autoconf' or 'none') [none] > > Available network interfaces are: vio0 vlan0. > > It doesn't seem right to offer vio0 in the list if it was already > configured via lladdr? The installer currently allows re-configuring interfaces and deletes the hostname.if before doing it. I think the most useful solution is to check for "the other" configuration for that interface and remove it as well when reconfiguring. > (Brushing aside the issue that fe:e1:b[ab]:dX:XX:XX addresses > are usually random so the hostname.lladdr scheme won't work for > them anyway..) This is true, but testing stuff in vmm is so handy!
Re: match ATI unknown as amdgpu in fw_update(8)
OK with me.
Jonathan Gray wrote:
> pci ids for newer amdgpu parts may not be known as all non-radeon ati
> display ids are matched in newer versions of amdgpu.
>
> in dmesg unknown products take the form:
> vga1 at pci12 dev 0 function 0 vendor "ATI", unknown product 0x687f rev 0xc3
> vendor "ATI", unknown product 0x687f (class display subclass VGA, rev 0x03)
> at pci12 dev 0 function 0 not configured
> amdgpu0 at pci12 dev 0 function 0 vendor "ATI", unknown product 0x687f rev
> 0xc3
>
> The diff below will match when an unknown azalia device or other ATI
> device is present. Alternatively it could be more specific:
> ^vga*vendor "ATI", unknown product
> ^vendor "ATI", unknown product*class display
>
> ^amdgpu*"ATI", unknown product
> should already be covered by the "amdgpu" line of firmware_patterns
>
> Index: patterns.c
> ===
> RCS file: /cvs/src/usr.sbin/fw_update/patterns.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 patterns.c
> --- patterns.c17 Nov 2022 13:30:21 - 1.5
> +++ patterns.c7 Dec 2022 00:44:39 -
> @@ -90,6 +90,7 @@ main(void)
> printf("%s\n", "acx");
> printf("%s\n", "amdgpu");
> print_devices("amdgpu", amdgpu_devices, nitems(amdgpu_devices));
> + printf("%s \"ATI\", unknown product\n", "amdgpu");
> printf("%s\n", "apple-boot ^cpu0*Apple");
> printf("%s\n", "athn");
> printf("%s\n", "bwfm");
>
Re: Get rid of UVM_VNODE_CANPERSIST
On Wed, Dec 07, 2022 at 03:56:56PM +0100, Alexander Bluhm wrote: > On Mon, Nov 28, 2022 at 03:04:17PM +0100, Mark Kettenis wrote: > > So here is an updated diff that checks the UVM_VNODE_DYING flag and > > skips the refcount manipulation if it is set. > > My macppc has build a full release with it. So it seems to fix the > issue. I will continue testing. I have run a few days of snapshot builds in a loop on my m1 mini and had this diff in my tree since it was posted on the list. The machine has been stable as usual. Earlier attempts to solve this problem would have blown up under this treatment.
Re: Get rid of UVM_VNODE_CANPERSIST
> We should try and commit this. I don't want to do this as a snapshot diff. So yes, that feels like the right approach.
Re: Document 'uidinfo' structure locks
On Thu, Dec 01, 2022 at 09:50:47PM +0300, Vitaliy Makkoveev wrote:
You could also document these globals with [I] in kern_proc.c
LIST_HEAD(uihashhead, uidinfo) *uihashtbl;
u_long uihash; /* size of hash table - 1 */
OK bluhm@
> Index: sys/sys/proc.h
> ===
> RCS file: /cvs/src/sys/sys/proc.h,v
> retrieving revision 1.335
> diff -u -p -r1.335 proc.h
> --- sys/sys/proc.h23 Nov 2022 11:00:27 - 1.335
> +++ sys/sys/proc.h1 Dec 2022 18:48:47 -
> @@ -303,6 +303,7 @@ struct p_inentry {
> * Locks used to protect struct members in this file:
> * I immutable after creation
> * S scheduler lock
> + * U uidinfolk
> * l read only reference, see lim_read_enter()
> * o owned (read/modified only) by this thread
> */
> @@ -433,10 +434,10 @@ struct proc {
> #ifdef _KERNEL
>
> struct uidinfo {
> - LIST_ENTRY(uidinfo) ui_hash;
> - uid_t ui_uid;
> - longui_proccnt; /* proc structs */
> - longui_lockcnt; /* lockf structs */
> + LIST_ENTRY(uidinfo) ui_hash;/* [U] */
> + uid_t ui_uid; /* [I] */
> + longui_proccnt; /* [U] proc structs */
> + longui_lockcnt; /* [U] lockf structs */
> };
>
> struct uidinfo *uid_find(uid_t);
Re: Get rid of UVM_VNODE_CANPERSIST
On Mon, Nov 28, 2022 at 03:04:17PM +0100, Mark Kettenis wrote:
> So here is an updated diff that checks the UVM_VNODE_DYING flag and
> skips the refcount manipulation if it is set.
My macppc has build a full release with it. So it seems to fix the
issue. I will continue testing.
I also made a performance comparison with kettenis@ and mpi@ diff.
http://bluhm.genua.de/perform/results/2022-12-05T17:42:17Z/perform.html
The only relevant part is the make-bsd-j8 test. There I see 2%
less system time with kettenis@ diff. So building kernel is faster.
But such small differences have to be interpreted with care.
> What do you think?
We should try and commit this.
bluhm
> Index: uvm/uvm_vnode.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v
> retrieving revision 1.130
> diff -u -p -r1.130 uvm_vnode.c
> --- uvm/uvm_vnode.c 20 Oct 2022 13:31:52 - 1.130
> +++ uvm/uvm_vnode.c 28 Nov 2022 14:01:20 -
> @@ -899,11 +899,21 @@ uvn_cluster(struct uvm_object *uobj, vof
> int
> uvn_put(struct uvm_object *uobj, struct vm_page **pps, int npages, int flags)
> {
> - int retval;
> + struct uvm_vnode *uvn = (struct uvm_vnode *)uobj;
> + int dying, retval;
>
> KASSERT(rw_write_held(uobj->vmobjlock));
>
> + dying = (uvn->u_flags & UVM_VNODE_DYING);
> + if (!dying) {
> + if (vget(uvn->u_vnode, LK_NOWAIT))
> + return VM_PAGER_AGAIN;
> + }
> +
> retval = uvn_io((struct uvm_vnode*)uobj, pps, npages, flags, UIO_WRITE);
> +
> + if (!dying)
> + vrele(uvn->u_vnode);
>
> return retval;
> }
>
>
Re: Configure interface by lladdr during install
On 2022/12/06 19:57, Andrew Hewus Fresh wrote: > Which interface do you wish to configure? (name, lladdr, '?', or 'done') > [vio0] ? > Available network interfaces are: vio0 vlan0. > vio0: lladdr fe:e1:bb:d1:dd:97 > Which interface do you wish to configure? (name, lladdr, '?', or 'done') > [vio0] fe:e1:bb:d1:dd:97 > IPv4 address for vio0? (or 'autoconf' or 'none') [autoconf] > IPv6 address for vio0? (or 'autoconf' or 'none') [none] > Available network interfaces are: vio0 vlan0. It doesn't seem right to offer vio0 in the list if it was already configured via lladdr? (Brushing aside the issue that fe:e1:b[ab]:dX:XX:XX addresses are usually random so the hostname.lladdr scheme won't work for them anyway..)
