add simplepmbus(4)
Add a driver for "simple-pm-bus" a way to enable a clock and/or
power domain for a group of devices.
https://www.kernel.org/doc/Documentation/devicetree/bindings/bus/simple-pm-bus.txt
This is needed to use am335x/omap4 dtbs from linux 5.11.
Index: sys/dev/fdt/files.fdt
===
RCS file: /cvs/src/sys/dev/fdt/files.fdt,v
retrieving revision 1.146
diff -u -p -r1.146 files.fdt
--- sys/dev/fdt/files.fdt 5 Feb 2021 00:05:20 - 1.146
+++ sys/dev/fdt/files.fdt 17 Feb 2021 00:49:02 -
@@ -23,6 +23,10 @@ device simplepanel
attach simplepanel at fdt
file dev/fdt/simplepanel.c simplepanel
+device simplepmbus: fdt
+attach simplepmbus at fdt
+file dev/fdt/simplepmbus.c simplepmbus
+
device sxiccmu
attach sxiccmu at fdt
file dev/fdt/sxiccmu.c sxiccmu
Index: share/man/man4/Makefile
===
RCS file: /cvs/src/share/man/man4/Makefile,v
retrieving revision 1.792
diff -u -p -r1.792 Makefile
--- share/man/man4/Makefile 4 Feb 2021 16:25:38 - 1.792
+++ share/man/man4/Makefile 17 Feb 2021 00:49:02 -
@@ -72,7 +72,8 @@ MAN= aac.4 abcrtc.4 abl.4 ac97.4 acphy.4
rl.4 rlphy.4 route.4 rsu.4 rtsx.4 rum.4 run.4 rtw.4 rtwn.4 \
safe.4 safte.4 sbus.4 schsio.4 scsi.4 sd.4 \
sdmmc.4 sdhc.4 se.4 ses.4 sf.4 sili.4 \
- simpleamp.4 simpleaudio.4 simplefb.4 simplepanel.4 siop.4 sis.4 sk.4 \
+ simpleamp.4 simpleaudio.4 simplefb.4 simplepanel.4 simplepmbus.4 \
+ siop.4 sis.4 sk.4 \
sm.4 smsc.4 softraid.4 spdmem.4 sdtemp.4 speaker.4 sppp.4 sqphy.4 \
ssdfb.4 st.4 ste.4 stge.4 sti.4 stp.4 sv.4 switch.4 sxiccmu.4 \
sxidog.4 sximmc.4 sxipio.4 sxipwm.4 sxirsb.4 sxirtc.4 sxisid.4 \
Index: sys/arch/armv7/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/armv7/conf/GENERIC,v
retrieving revision 1.134
diff -u -p -r1.134 GENERIC
--- sys/arch/armv7/conf/GENERIC 4 Feb 2021 16:25:39 - 1.134
+++ sys/arch/armv7/conf/GENERIC 17 Feb 2021 00:49:02 -
@@ -31,6 +31,7 @@ configbsd swap generic
# The main bus device
mainbus0 at root
simplebus* at fdt?
+simplepmbus* at fdt?
cpu0 at mainbus?
# Cortex-A9
Index: sys/arch/armv7/conf/RAMDISK
===
RCS file: /cvs/src/sys/arch/armv7/conf/RAMDISK,v
retrieving revision 1.119
diff -u -p -r1.119 RAMDISK
--- sys/arch/armv7/conf/RAMDISK 18 Jun 2020 08:48:04 - 1.119
+++ sys/arch/armv7/conf/RAMDISK 17 Feb 2021 00:49:02 -
@@ -30,6 +30,7 @@ configbsd root on rd0a swap on rd0b
mainbus0 at root
softraid0 at root
simplebus* at fdt?
+simplepmbus* at fdt?
cpu0 at mainbus?
# Cortex-A9
--- /dev/null Wed Feb 17 11:49:05 2021
+++ sys/dev/fdt/simplepmbus.c Tue Feb 16 17:24:55 2021
@@ -0,0 +1,62 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2021 Jonathan Gray
+ *
+ * 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.
+ */
+
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+
+#include
+
+intsimplepmbus_match(struct device *, void *, void *);
+void simplepmbus_attach(struct device *, struct device *, void *);
+
+struct simplepmbus_softc {
+ struct simplebus_softc sc_bus;
+};
+
+struct cfattach simplepmbus_ca = {
+ sizeof(struct simplepmbus_softc), simplepmbus_match, simplepmbus_attach
+};
+
+struct cfdriver simplepmbus_cd = {
+ NULL, "simplepmbus", DV_DULL
+};
+
+int
+simplepmbus_match(struct device *parent, void *cfdata, void *aux)
+{
+ struct fdt_attach_args *faa = aux;
+
+ return OF_is_compatible(faa->fa_node, "simple-pm-bus");
+}
+
+void
+simplepmbus_attach(struct device *parent, struct device *self, void *aux)
+{
+ struct simplepmbus_softc *sc = (struct simplepmbus_softc *)self;
+ struct fdt_attach_args *faa = aux;
+
+ power_domain_enable(faa->fa_node);
+ clock_enable_all(faa->fa_node);
+
+ simplebus_attach(parent, >sc_bus.sc_dev, faa);
+}
--- /dev/null Wed Feb 17 11:49:07 2021
+++ share/man/man4/simplepmbus.4Wed Feb 17 11:31:06 2021
@@ -0,0 +1,36
Re: [PATCH] fixes relayd Websocket "Connection: close" header when Upgrade is requested
On 2021/02/16 20:12, Franz Bettag wrote: > My point, the protocol after HTTP 1.0 encourages keep-alives anyway. > Close is only default in 1.0 so basically you wouldn’t have lingering > dead sockets on your server. If you want a full featured HTTP implementation then perhaps relayd is not for you. It is an amalgamation of a web firewall type of thing with a load balancer / dead backend detection so any diff needs to consider all use cases. It is not ready to handle multiple requests in one connection, it makes a decision about where to send the request then gets out the way (handing the packet-shovelling over to the kernel via "splice"). To handle persistent connections it would need to stay in the loop for future requests and process them separately, maybe to a different backend. Anyway I am not all that interested in relayd http proxying so I will drop out of this thread, just trying to give a clue about what might be needed in order to move this ahead. I am not saying the diff *is* problematic but you can bet anyone committing it will need to satisfy themselves that it doesn't introduce a new problem and there has not been discussion of that in the thread afaicr.
Re: LibreSSL regressions
On Tue, Feb 16, 2021 at 01:16:21PM +0100, Jan Klemkow wrote: > On Tue, Feb 16, 2021 at 04:36:59AM +1100, Joel Sing wrote: > > On 21-02-15 14:49:46, Jan Klemkow wrote: > > > On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote: > > > > On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote: > > > > > A coworker of mine has made tests with LibreSSL [1] and found some > > > > > regressions. I took his test descriptions and created the following > > > > > automated regression test. In the repository he described his > > > > > findings > > > > > in detail. I kept the numbers of the files and subtests in the target > > > > > names for now. So, its easier to match it with his files. > > > > > > > > > > I don't know how to handle the result of "test-01-ssl". Thats why its > > > > > just a comment. Someone may have an idea to handle this properly. > > > > > > > > > > Any comments, wishes or OK's? > > > > > > > > > > [1]: https://github.com/noxxi/libressl-tests > > > > > > > > First of all thanks for the effort! > > > > > > > > The perl script and probably also the Makefile should have a license. > > > > > > > > Please add a check that tests whether the required perl modules are > > > > installed (p5-IO-Socket-SSL and p5-Net-SSLeay) and otherwise prints > > > > SKIPPED and their names, so I can install them if they're not present. > > > > I never remember their exact capitalization and hyphenation... > > > > > > > > Various comments inline, and a patch for openssl(1) at the end that may > > > > simplify some things. > > > > > > This is an updated version of the test including comments and wishes > > > from tb@ and bluhm@. > > > > > > OK? > > > > This currently drives openssl(1) for tests, which means that it is > > testing openssl(1), libssl and libcrypto, when what you're really > > wanting to test is libcrypto's verifier. While this works, the > > problem is that a change or breakage in libssl or openssl(1) results > > in a regress failure for libcrypto. If this is to land in its > > current form it really should be in regress/usr.bin/openssl - > > alternatively, it could be reworked to explicitly test libcrypto's > > APIs and remain here. > > > > Some additional comments inline. > > So, the following diff should hit all needs. > > OK? Yes, it's ok. Thanks. I played a bit with it, and it is a bit more fragile than I would like, but we may fix that in tree or redo some of it without using openssl(1) later. The ground covered is definitely worthwhile to have in regres, so please go ahead. (jsing has no objection).
Re: [PATCH] fixes relayd Websocket "Connection: close" header when Upgrade is requested
Clearly the current behaviour with two Connection headers is wrong and this may be a correct diff, but the description accompanying it only considers "how does this fix the problem seen" rather than "what are the implications of this, does it do anything bad?". I'm only making very minimal use of relayd and IIRC not with relay_http, but the thing this makes me wonder about is, what is the "Connection: close" for? If it's there to prevent clients from smuggling a follow-on request past relayd to the server without going through the filter rules then we need to be sure that dropping the "Connection: close" in this case doesn't bypass that. On 2021/02/16 14:25, Franz Bettag wrote: > At least someone besides me cares about correct protocols in base daemons :] > > Sent from my iPhone > > > On 14 Feb 2021, at 12:06, Marcus MERIGHI wrote: > > > > Another month has passed, another friendly bump... > > > > patch against -current attached, for convenience... > > > > Marcus > > > > Index: relay_http.c > > === > > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > > retrieving revision 1.80 > > diff -u -p -u -r1.80 relay_http.c > > --- relay_http.c9 Jan 2021 08:53:58 -1.80 > > +++ relay_http.c14 Feb 2021 11:03:12 - > > @@ -527,7 +527,7 @@ relay_read_http(struct bufferevent *bev, > > * Ask the server to close the connection after this request > > * since we don't read any further request headers. > > */ > > -if (cre->toread == TOREAD_UNLIMITED) > > +if (cre->toread == TOREAD_UNLIMITED && upgrade == NULL) > >if (kv_add(>http_headers, "Connection", > >"close", 0) == NULL) > >goto fail; > > > > mcmer-open...@tor.at (Marcus MERIGHI), 2021.01.04 (Mon) 12:59 (CET): > >> One month has passed, this is just a friendly ping... > >> > >> Marcus > >> > >> mcmer-open...@tor.at (Marcus MERIGHI), 2020.12.04 (Fri) 14:18 (CET): > >>> This patch wasn't commited and not discussed (publicly). > >>> > >>> It lets me use relayd as a front-end to the mattermost-server. > >>> > >>> @franz: Thank you! > >>> > >>> fr...@bett.ag (Franz Bettag), 2020.03.04 (Wed) 17:52 (CET): > After migrating my home setup from nginx reverse proxying to relayd, i > noticed my iOS devices having issues connecting through Websockets. > After debugging, i noticed that relayd adds the "Connection: close" > regardless of upgrade being requested. > > This issue is also reported on a blog-post using relayd as a Websocket > Client. https://deftly.net/posts/2019-10-23-websockets-with-relayd.html > > This resulted in the response having two Connection Headers, one > "Connection: upgrade" and one "Connection: close", which iOS strict > implementation does not allow. > > I have fixed the if condition that leads to this issue. > > The fix has been tested with Apple iOS 13 and the problem can be > observed using Firefox and just connecting to something Websocket over > relayd. Both "Connection:" headers will appear. > > best regards > > Franz > > > diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c > index 960d4c54a..3a6678790 100644 > --- usr.sbin/relayd/relay_http.c > +++ usr.sbin/relayd/relay_http.c > @@ -524,9 +524,11 @@ relay_read_http(struct bufferevent *bev, void *arg) > > /* > * Ask the server to close the connection after this request > - * since we don't read any further request headers. > + * since we don't read any further request headers, unless > + * an Upgrade is requested, in which case we do NOT want to add > + * this header. > */ > -if (cre->toread == TOREAD_UNLIMITED) > +if (cre->toread == TOREAD_UNLIMITED && upgrade == NULL) > if (kv_add(>http_headers, "Connection", > "close", 0) == NULL) > goto fail; > > >
Re: rw_enter_diag() vs WITNESS
On Tue, Feb 16, 2021 at 10:07:27AM +0100, Martin Pieuchot wrote:
> On 15/02/21(Mon) 16:58, Visa Hankala wrote:
> > On Mon, Feb 15, 2021 at 11:37:45AM +0100, Martin Pieuchot wrote:
> > > Diagnostic function rw_enter_diag() should be called before
> > > WITNESS_CHECKORDER() to have proper locking/debugging information.
> > >
> > > In the case of 'locking against myself' it is currently impossible
> > > to know where the lock has been previously acquired. Diff below fixes
> > > that, ok?
> >
> > Based on this description alone, it is not clear to me what exactly
> > gets solved. Doesn't the code reach rw_enter_diag() inside the loop
> > when trying to recurse on the rwlock?
>
> It does but before reaching WITNESS_CHECKORDER(). So if a panic is
> triggered "show all locks" will point to the previous place where the
> same lock has been acquired.
> Currently it points to the place triggering the panic which doesn't
> help figuring the problem.
Something is not right. "show all locks" should show the locks that are
currently being held by threads or CPUs. WITNESS_CHECKORDER() should
have no effect on those locks. Only WITNESS_LOCK() or WITNESS_UNLOCK()
should change the locked state.
So if there is an rwlock recursion, "show all locks", or "show locks",
should display where the thread did the initial rwlock acquisition (when
kern.witness.locktrace is enabled), and the panic's stack trace should
show where it attempted the second acquisition.
>
> > Does this change have implications with (panicstr || db_active)?
>
> Indeed, updated diff below.
>
> Index: kern/kern_rwlock.c
> ===
> RCS file: /cvs/src/sys/kern/kern_rwlock.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 kern_rwlock.c
> --- kern/kern_rwlock.c8 Feb 2021 08:18:45 - 1.47
> +++ kern/kern_rwlock.c16 Feb 2021 09:04:04 -
> @@ -167,6 +167,9 @@ rw_exit_write(struct rwlock *rwl)
> static void
> rw_enter_diag(struct rwlock *rwl, int flags)
> {
> + if (panicstr || db_active)
> + return;
> +
> switch (flags & RW_OPMASK) {
> case RW_WRITE:
> case RW_READ:
> @@ -237,7 +240,11 @@ rw_enter(struct rwlock *rwl, int flags)
> int error, prio;
> #ifdef WITNESS
> int lop_flags;
> +#endif
> +
> + rw_enter_diag(rwl, flags);
>
> +#ifdef WITNESS
> lop_flags = LOP_NEWORDER;
> if (flags & RW_WRITE)
> lop_flags |= LOP_EXCLUSIVE;
> @@ -270,8 +277,6 @@ retry:
> continue;
> }
> #endif
> -
> - rw_enter_diag(rwl, flags);
>
> if (flags & RW_NOSLEEP)
> return (EBUSY);
>
Re: LibreSSL regressions
On Tue, Feb 16, 2021 at 04:36:59AM +1100, Joel Sing wrote: > On 21-02-15 14:49:46, Jan Klemkow wrote: > > On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote: > > > On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote: > > > > A coworker of mine has made tests with LibreSSL [1] and found some > > > > regressions. I took his test descriptions and created the following > > > > automated regression test. In the repository he described his findings > > > > in detail. I kept the numbers of the files and subtests in the target > > > > names for now. So, its easier to match it with his files. > > > > > > > > I don't know how to handle the result of "test-01-ssl". Thats why its > > > > just a comment. Someone may have an idea to handle this properly. > > > > > > > > Any comments, wishes or OK's? > > > > > > > > [1]: https://github.com/noxxi/libressl-tests > > > > > > First of all thanks for the effort! > > > > > > The perl script and probably also the Makefile should have a license. > > > > > > Please add a check that tests whether the required perl modules are > > > installed (p5-IO-Socket-SSL and p5-Net-SSLeay) and otherwise prints > > > SKIPPED and their names, so I can install them if they're not present. > > > I never remember their exact capitalization and hyphenation... > > > > > > Various comments inline, and a patch for openssl(1) at the end that may > > > simplify some things. > > > > This is an updated version of the test including comments and wishes > > from tb@ and bluhm@. > > > > OK? > > This currently drives openssl(1) for tests, which means that it is > testing openssl(1), libssl and libcrypto, when what you're really > wanting to test is libcrypto's verifier. While this works, the > problem is that a change or breakage in libssl or openssl(1) results > in a regress failure for libcrypto. If this is to land in its > current form it really should be in regress/usr.bin/openssl - > alternatively, it could be reworked to explicitly test libcrypto's > APIs and remain here. > > Some additional comments inline. So, the following diff should hit all needs. OK? Thanks, Jan Index: usr.bin/openssl/Makefile === RCS file: /cvs/src/regress/usr.bin/openssl/Makefile,v retrieving revision 1.6 diff -u -p -r1.6 Makefile --- usr.bin/openssl/Makefile19 Mar 2018 03:41:40 - 1.6 +++ usr.bin/openssl/Makefile15 Feb 2021 20:37:11 - @@ -1,6 +1,6 @@ # $OpenBSD: Makefile,v 1.6 2018/03/19 03:41:40 beck Exp $ -SUBDIR= options +SUBDIR= options x509 CLEANFILES+= testdsa.key testdsa.pem rsakey.pem rsacert.pem dsa512.pem CLEANFILES+= appstest_dir Index: usr.bin/openssl/x509/Makefile === RCS file: usr.bin/openssl/x509/Makefile diff -N usr.bin/openssl/x509/Makefile --- /dev/null 1 Jan 1970 00:00:00 - +++ usr.bin/openssl/x509/Makefile 16 Feb 2021 12:06:10 - @@ -0,0 +1,129 @@ +# $OpenBSD$ + +# Copyright (c) 2021 Jan Klemkow +# +# 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 regression test is based on manual test descriptions from: +# https://github.com/noxxi/libressl-tests + +# The following port must be installed for the regression tests: +# p5-IO-Socket-SSL perl interface to SSL sockets + +PERL = perl +OPENSSL ?= openssl + +PKG_REQUIRE != pkg_info -e 'p5-IO-Socket-SSL-*' +.if empty (PKG_REQUIRE) +regress: + @echo "missing package p5-IO-Socket-SSL" + @echo SKIPPED +.endif + +REGRESS_TARGETS += test-inlabel-wildcard-cert-no-CA-client +REGRESS_TARGETS += test-inlabel-wildcard-cert-CA-client +REGRESS_TARGETS += test-common-wildcard-cert-no-CA-client +REGRESS_TARGETS += test-common-wildcard-cert-CA-client +REGRESS_TARGETS += test-verify-unusual-wildcard-cert +REGRESS_TARGETS += test-openssl-verify-common-wildcard-cert +REGRESS_TARGETS += test-chain-certificates-s_server +REGRESS_TARGETS += test-alternative-chain +REGRESS_CLEANUP = cleanup-ssl +REGRESS_SETUP_ONCE = create-libressl-test-certs + +REGRESS_EXPECTED_FAILURES += test-unusual-wildcard-cert-no-CA-client +REGRESS_EXPECTED_FAILURES += test-common-wildcard-cert-no-CA-client +REGRESS_EXPECTED_FAILURES +=
Re: [PATCH] fixes relayd Websocket "Connection: close" header when Upgrade is requested
At least someone besides me cares about correct protocols in base daemons :] Sent from my iPhone > On 14 Feb 2021, at 12:06, Marcus MERIGHI wrote: > > Another month has passed, another friendly bump... > > patch against -current attached, for convenience... > > Marcus > > Index: relay_http.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > retrieving revision 1.80 > diff -u -p -u -r1.80 relay_http.c > --- relay_http.c9 Jan 2021 08:53:58 -1.80 > +++ relay_http.c14 Feb 2021 11:03:12 - > @@ -527,7 +527,7 @@ relay_read_http(struct bufferevent *bev, > * Ask the server to close the connection after this request > * since we don't read any further request headers. > */ > -if (cre->toread == TOREAD_UNLIMITED) > +if (cre->toread == TOREAD_UNLIMITED && upgrade == NULL) >if (kv_add(>http_headers, "Connection", >"close", 0) == NULL) >goto fail; > > mcmer-open...@tor.at (Marcus MERIGHI), 2021.01.04 (Mon) 12:59 (CET): >> One month has passed, this is just a friendly ping... >> >> Marcus >> >> mcmer-open...@tor.at (Marcus MERIGHI), 2020.12.04 (Fri) 14:18 (CET): >>> This patch wasn't commited and not discussed (publicly). >>> >>> It lets me use relayd as a front-end to the mattermost-server. >>> >>> @franz: Thank you! >>> >>> fr...@bett.ag (Franz Bettag), 2020.03.04 (Wed) 17:52 (CET): After migrating my home setup from nginx reverse proxying to relayd, i noticed my iOS devices having issues connecting through Websockets. After debugging, i noticed that relayd adds the "Connection: close" regardless of upgrade being requested. This issue is also reported on a blog-post using relayd as a Websocket Client. https://deftly.net/posts/2019-10-23-websockets-with-relayd.html This resulted in the response having two Connection Headers, one "Connection: upgrade" and one "Connection: close", which iOS strict implementation does not allow. I have fixed the if condition that leads to this issue. The fix has been tested with Apple iOS 13 and the problem can be observed using Firefox and just connecting to something Websocket over relayd. Both "Connection:" headers will appear. best regards Franz diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c index 960d4c54a..3a6678790 100644 --- usr.sbin/relayd/relay_http.c +++ usr.sbin/relayd/relay_http.c @@ -524,9 +524,11 @@ relay_read_http(struct bufferevent *bev, void *arg) /* * Ask the server to close the connection after this request - * since we don't read any further request headers. + * since we don't read any further request headers, unless + * an Upgrade is requested, in which case we do NOT want to add + * this header. */ -if (cre->toread == TOREAD_UNLIMITED) +if (cre->toread == TOREAD_UNLIMITED && upgrade == NULL) if (kv_add(>http_headers, "Connection", "close", 0) == NULL) goto fail; >
Re: wsmouse(4): allow independent touchpad tap gesture configuration
> Hi,
>
> I wouldn't mind to see such a feature in wsmouse, but there are two
> things that I don't like about the patch in this form: 1) It breaks
> existing configurations, ...
I know sometimes breaking changes are made in the name of forward
progress, and I wasn't sure if this was one of those times.
> ... and 2) it may bother the users who don't care about such an
> extension - probably the majority of those who enable tapping.
>
> A better approach might be to extend wsconsctl in such a way that
> it accepts both a single value as well as a triple of values as
> input, and prints them in alternating formats, depending on whether
> all values are the same or not.
While I handle single value assignment, I defer on printing. I worked
on it some, but it turned into a real mess. I also don't see much harm
in printing the full triple. Users don't see it except when it is
assigned during boot and when using wsconsctl, so it shouldn't be an
annoyance. And it may also serve as an indicator that something has
changed with regards to touchpad features, prompting investigation.
> (Moreover, if we add special handlers for the 'tapping' field in
> wsconsctrl, then it's possible to use the WSMOUSECFG_TAPPING
> parameter as a bit field, which might be more adequate here.)
This would require a few changes on the kernel side. Right now,
WSTPAD_TAPPING is a single bit within wstpad.features, which is why I
added two more flags. To store the configuration, adding a "tapping"
field to wstpad.params makes the most sense (to me). It wouldn't be too
much work to transition to this layout, and this has the advantage of
keeping all of the tapping configuration parameters together in one
place instead of as three feature flags. I assume that the user would
still see a triple and not the underlying integer representation when
configuring the touchpad via wsconsctl.
Below you will find my updated diff, which adds single value assignment
to wsconsctl for mouse.tp.tapping. I have also updated the man page to
reflect this change. However, it sounds like more changes are in store,
so I will wait for your feedback on what to implement.
Index: sbin/wsconsctl/mousecfg.c
===
RCS file: /cvs/src/sbin/wsconsctl/mousecfg.c,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 mousecfg.c
--- sbin/wsconsctl/mousecfg.c 2 Apr 2020 17:17:04 - 1.7
+++ sbin/wsconsctl/mousecfg.c 16 Feb 2021 08:58:27 -
@@ -40,7 +40,7 @@
#define TP_FILTER_FIRSTWSMOUSECFG_DX_MAX
#define TP_FILTER_LAST WSMOUSECFG_SMOOTHING
#define TP_FEATURES_FIRST WSMOUSECFG_SOFTBUTTONS
-#define TP_FEATURES_LAST WSMOUSECFG_TAPPING
+#define TP_FEATURES_LAST WSMOUSECFG_THREEFINGERTAPPING
#define TP_SETUP_FIRST WSMOUSECFG_LEFT_EDGE
#define TP_SETUP_LAST WSMOUSECFG_TAP_LOCKTIME
#define LOG_FIRST WSMOUSECFG_LOG_INPUT
@@ -71,8 +71,10 @@ static const int touchpad_types[] = {
struct wsmouse_parameters cfg_tapping = {
(struct wsmouse_param[]) {
- { WSMOUSECFG_TAPPING, 0 }, },
- 1
+ { WSMOUSECFG_ONEFINGERTAPPING, 0 },
+ { WSMOUSECFG_TWOFINGERTAPPING, 0 },
+ { WSMOUSECFG_THREEFINGERTAPPING, 0 }, },
+ 3
};
struct wsmouse_parameters cfg_scaling = {
@@ -262,6 +264,24 @@ set_percent(struct wsmouse_parameters *f
}
static int
+set_tapping(struct wsmouse_parameters *field, char *tapping)
+{
+ int i1, i2, i3;
+
+ switch (sscanf(tapping, "%d,%d,%d", , , )) {
+ case 1:
+ i2 = i3 = i1;
+ /* FALLTHROUGH */
+ case 3:
+ set_value(field, WSMOUSECFG_ONEFINGERTAPPING, i1);
+ set_value(field, WSMOUSECFG_TWOFINGERTAPPING, i2);
+ set_value(field, WSMOUSECFG_THREEFINGERTAPPING, i3);
+ return (0);
+ }
+ return (-1);
+}
+
+static int
set_edges(struct wsmouse_parameters *field, char *edges)
{
float f1, f2, f3, f4;
@@ -359,6 +379,12 @@ mousecfg_rd_field(struct wsmouse_paramet
if (field == _param) {
if (read_param(field, val))
errx(1, "invalid input (param)");
+ return;
+ }
+
+ if (field == _tapping) {
+ if (set_tapping(field, val))
+ errx(1, "invalid input (tapping)");
return;
}
Index: share/man/man4/wsmouse.4
===
RCS file: /cvs/src/share/man/man4/wsmouse.4,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 wsmouse.4
--- share/man/man4/wsmouse.44 Feb 2018 20:29:59 - 1.20
+++ share/man/man4/wsmouse.416 Feb 2021 08:58:27 -
@@ -26,7 +26,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd $Mdocdate: February 4 2018 $
+.Dd $Mdocdate: February 15 2021 $
.Dt WSMOUSE 4
.Os
.Sh NAME
@@ -87,14 +87,23
UNIX domain sockets: move garbage collector to `systqmp'
There are no fallout reports after UNIX sockets unlocking, so I propose
to move moves garbage collector to `systqmp'. unp_gc() touches nothing
which requires kernel lock to be held.
Index: sys/kern/uipc_usrreq.c
===
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.143
diff -u -p -r1.143 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c 10 Feb 2021 08:20:09 - 1.143
+++ sys/kern/uipc_usrreq.c 16 Feb 2021 09:44:26 -
@@ -444,7 +444,7 @@ unp_detach(struct unpcb *unp)
m_freem(unp->unp_addr);
pool_put(_pool, unp);
if (unp_rights)
- task_add(systq, _gc_task);
+ task_add(systqmp, _gc_task);
if (vp != NULL) {
/*
@@ -1197,7 +1197,7 @@ unp_discard(struct fdpass *rp, int nfds)
memset(rp, 0, sizeof(*rp) * nfds);
SLIST_INSERT_HEAD(_deferred, defer, ud_link);
- task_add(systq, _gc_task);
+ task_add(systqmp, _gc_task);
}
int
Re: Posted vs. non-posted device access
> From: David Gwynne > Date: Tue, 16 Feb 2021 12:58:35 +1000 > > > On 16 Feb 2021, at 06:01, Mark Kettenis wrote: > > > >> Date: Mon, 15 Feb 2021 01:19:29 +0100 > >> From: Patrick Wildt > >> > >> Am Mon, Feb 15, 2021 at 09:55:56AM +1000 schrieb David Gwynne: > >>> > >>> > On 15 Feb 2021, at 07:54, Mark Kettenis wrote: > > One of the aspects of device access is whether CPU writes to a device > are posted or non-posted. For non-posted writes, the CPU will wait > for the device to acknowledge that the write has performed. If the > device sits on a bus far away, this can take a while and slow things > down. The alternative are so-called posted writes. The CPU will > "post" the write to the bus without waiting for an acknowledgement. > The CPU may receive an asynchronous notifaction at a later time that > the write didn't succeed or a failing write may be dropped without > further botification. On most architectures whether writes are posted > or not is a property of the bus between the CPU and the device. For > example, memory mapped I/O on the PCI bus is always posted and there > is nothing the CPU can do about it. > > On the ARM architecture though we can indicate to the CPU whether > writes to a certain address range should be posted or not. This is > done by specifying certain memory attributes in the mappings used by > the MMU. The OpenBSD kernel always specifies device access as > non-posted. On all ARM implementations we have seen so far this seems > to work even for writes to devices connected to a PCIe bus. There > might be a penalty though, so I need to investigate this a bit > further. > > However, on Apple's M1 SoC, this isn't the case. Non-posted writes to > a bus that uses posted writes fail and vice-versa. So in order to use > the PCIe bus on these SoCs we need to specify the right memory > attributes. The diff below implements this by introducing a new > BUS_SPACE_MAP_POSTED flag. At this point I don't expect generic > drivers to use this flag yet. So there is no need to add it for other > architectures. But I don't rule out we may have to use this flag in > sys/dev/fdt sometime in the future. That is why I posted this to a > wider audience. > >>> > >>> You don't want to (ab)use one of the existing flags? If I squint > >>> and read kind of quickly I could imagine this is kind of like > >>> write combining, like what BUS_SPACE_MAP_PREFETCHABLE can do on > >>> pci busses. > >> > >> BUS_SPACE_MAP_PREFETCHABLE should be "normal uncached" memory on arm64, > >> which is different to device memory. That said I have a device where > >> amdgpu(4) doesn't behave if it's "normal uncached", and I'm not sure if > >> it's the HW's fault or if there's some barrier missing. Still, I would > >> not use BUS_SPACE_MAP_PREFETCHABLE for nGnRnE vs nGnRE. > >> > >> More info on device vs normal is here: > >> > >> https://developer.arm.com/documentation/102376/0100/Normal-memory > >> https://developer.arm.com/documentation/102376/0100/Device-memory > > > > BUS_SPACE_MAP_PREFETCHABLE is used for parts of the address space that > > are "side-effect free". That means that multiple writes may be > > combined into one and reads might actually fetch more data than you > > asked for. Typical use is a frambuffer or some other device memory > > that is accessed across a PCI bus. In most cases that is not what you > > want to access device registers where a read or a write triggers some > > action in the device. > > > > Posted writes still happen as issued (and in principle in the same > > order as issued). But they may complete at a later time after the CPU > > has executed many more instructions. The traditional way to make sure > > posted writes on a PCI bus have completed is to read something back > > from the device. > > > > So as Patrick said, it's a different thing. > > > >>> If this does leak into fdt, would it just be a nop on other archs > >>> that use those drivers? > > > > Most likely, yes. All other architectures that I know have don't > > require the CPU to do someting different for posted and non-posted > > writes. > > fair enough. maybe whether the bus is posted or not could be part of > the fdt/acpi info, rather than something hardcoded in drivers? There is a proposal to do just that. But it means bus drivers still need to worry about this and I still need a flag. In the current implementation (that I didn't include) the PCIe host bridge driver for the Apple M1 will take care of setting the flag in its bus_space_map() implementation. I don't expect "normal" device drivers to have to care about this. > anyway, i'm ok with this as is so go for it. Thanks Mark > > ok? > > > Index: arch/arm64/arm64/locore.S >
Re: bgpd rde decide all for route-servers
On Tue, Feb 16, 2021 at 10:44:31AM +0100, Claudio Jeker wrote:
> On route-servers at IXPs there is often the wish to not have path hiding
> because of output filters. On way of solving this is to use per peer RIBs
> but the cost (in memory) is very high. This solves this issue in a
> different way but with the same (or similar result).
>
> In bgpd all prefixes/paths are sorted by their preference and by default
> the best path is put into the Adj-RIB-Out (after that prefix passed the
> output filter). This diff changes this behaviour by falling back to the
> next prefix in case the current prefix was filtered. This stops path
> hiding and announces the next best route. The cost for doing this is a
> bump in CPU usage. On the other hand the memory consumption remains the
> same.
>
> The feature can be enabled with 'rde evaluate all' globably or per peer.
> It is also to turn it off by using 'rde evaluate default' (in case a peer
> does not want this).
Better diff that fixes an issue in parse.y adding DEFAULT as keyword.
--
:wq Claudio
Index: bgpd.conf.5
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.208
diff -u -p -r1.208 bgpd.conf.5
--- bgpd.conf.5 16 Feb 2021 08:29:16 - 1.208
+++ bgpd.conf.5 16 Feb 2021 09:22:37 -
@@ -268,9 +268,18 @@ daemons, such as
.Xr ospfd 8 .
.Pp
.It Xo
-.Ic rde
-.Ic med
-.Ic compare
+.Ic rde Ic evaluate
+.Pq Ic default Ns | Ns Ic all
+.Xc
+If set to
+.Ar all
+keep evaluating alternative paths in case the selected path is filtered
+out.
+By default if a path is filtered by the output filters then no alternative
+path is sent to this peer.
+.Pp
+.It Xo
+.Ic rde Ic med Ic compare
.Pq Ic always Ns | Ns Ic strict
.Xc
If set to
@@ -1151,6 +1160,20 @@ setting.
.Pp
.It Ic remote-as Ar as-number
Set the AS number of the remote system.
+.Pp
+.It Xo
+.Ic rde Ic evaluate
+.Pq Ic default Ns | Ns Ic all
+.Xc
+If set to
+.Ar all
+keep evaluating alternative paths in case the selected path is filtered
+out.
+By default if a path is filtered by the output filters then no alternative
+path is sent to this peer.
+The default is inherited from the global
+.Ic rde Ic evaluate
+setting.
.Pp
.It Ic rib Ar name
Bind the neighbor to the specified RIB.
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.412
diff -u -p -r1.412 bgpd.h
--- bgpd.h 16 Feb 2021 08:29:16 - 1.412
+++ bgpd.h 16 Feb 2021 09:22:38 -
@@ -65,7 +65,8 @@
#defineBGPD_FLAG_DECISION_ROUTEAGE 0x0100
#defineBGPD_FLAG_DECISION_TRANS_AS 0x0200
#defineBGPD_FLAG_DECISION_MED_ALWAYS 0x0400
-#defineBGPD_FLAG_NO_AS_SET 0x0800
+#defineBGPD_FLAG_DECISION_ALL_PATHS0x0800
+#defineBGPD_FLAG_NO_AS_SET 0x1000
#defineBGPD_LOG_UPDATES0x0001
@@ -408,7 +409,8 @@ struct peer_config {
#define PEERFLAG_TRANS_AS 0x01
#define PEERFLAG_LOG_UPDATES 0x02
-#define PEERFLAG_NO_AS_SET 0x04
+#define PEERFLAG_EVALUATE_ALL 0x04
+#define PEERFLAG_NO_AS_SET 0x08
enum network_type {
NETWORK_DEFAULT,/* from network statements */
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.413
diff -u -p -r1.413 parse.y
--- parse.y 16 Feb 2021 08:29:16 - 1.413
+++ parse.y 16 Feb 2021 10:28:39 -
@@ -791,6 +791,19 @@ conf_main : AS as4number {
}
free($4);
}
+ | RDE EVALUATE STRING {
+ if (!strcmp($3, "all"))
+ conf->flags |= BGPD_FLAG_DECISION_ALL_PATHS;
+ else if (!strcmp($3, "default"))
+ conf->flags &= ~BGPD_FLAG_DECISION_ALL_PATHS;
+ else {
+ yyerror("rde evaluate: "
+ "unknown setting \"%s\"", $3);
+ free($3);
+ YYERROR;
+ }
+ free($3);
+ }
| NEXTHOP QUALIFY VIA STRING{
if (!strcmp($4, "bgp"))
conf->flags |= BGPD_FLAG_NEXTHOP_BGP;
@@ -1727,6 +1740,19 @@ peeropts : REMOTEAS as4number{
else
curpeer->conf.flags &= ~PEERFLAG_NO_AS_SET;
}
+ | RDE EVALUATE STRING {
+ if (!strcmp($3, "all"))
+ curpeer->conf.flags |= PEERFLAG_EVALUATE_ALL;
+ else if (!strcmp($3, "default"))
+ curpeer->conf.flags &= ~PEERFLAG_EVALUATE_ALL;
+
uvm_fault_lower refactoring
Start by moving `pgo_fault' handler outside of uvm_fault_lower().
If a page has a backing object that prefer to handler to fault itself
the locking will be different, so keep it under KERNEL_LOCK() for the
moment and make it separate from the rest of uvm_fault_lower().
ok?
Index: uvm/uvm_fault.c
===
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
retrieving revision 1.115
diff -u -p -r1.115 uvm_fault.c
--- uvm/uvm_fault.c 16 Feb 2021 09:10:17 - 1.115
+++ uvm/uvm_fault.c 16 Feb 2021 10:13:58 -
@@ -598,10 +598,37 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
/* case 1: fault on an anon in our amap */
error = uvm_fault_upper(, , anons, fault_type);
} else {
- /* case 2: fault on backing object or zero fill */
- KERNEL_LOCK();
- error = uvm_fault_lower(, , pages, fault_type);
- KERNEL_UNLOCK();
+ struct uvm_object *uobj = ufi.entry->object.uvm_obj;
+
+ /*
+* if the desired page is not shadowed by the amap and
+* we have a backing object, then we check to see if
+* the backing object would prefer to handle the fault
+* itself (rather than letting us do it with the usual
+* pgo_get hook). the backing object signals this by
+* providing a pgo_fault routine.
+*/
+ if (uobj != NULL && uobj->pgops->pgo_fault != NULL) {
+ KERNEL_LOCK();
+ error = uobj->pgops->pgo_fault(,
+ flt.startva, pages, flt.npages,
+ flt.centeridx, fault_type, flt.access_type,
+ PGO_LOCKED);
+ KERNEL_UNLOCK();
+
+ if (error == VM_PAGER_OK)
+ error = 0;
+ else if (error == VM_PAGER_REFAULT)
+ error = ERESTART;
+ else
+ error = EACCES;
+ } else {
+ /* case 2: fault on backing obj or zero fill */
+ KERNEL_LOCK();
+ error = uvm_fault_lower(, , pages,
+ fault_type);
+ KERNEL_UNLOCK();
+ }
}
}
@@ -1054,26 +1081,6 @@ uvm_fault_lower(struct uvm_faultinfo *uf
struct vm_anon *anon = NULL;
vaddr_t currva;
voff_t uoff;
-
- /*
-* if the desired page is not shadowed by the amap and we have a
-* backing object, then we check to see if the backing object would
-* prefer to handle the fault itself (rather than letting us do it
-* with the usual pgo_get hook). the backing object signals this by
-* providing a pgo_fault routine.
-*/
- if (uobj != NULL && uobj->pgops->pgo_fault != NULL) {
- result = uobj->pgops->pgo_fault(ufi, flt->startva, pages,
- flt->npages, flt->centeridx, fault_type, flt->access_type,
- PGO_LOCKED);
-
- if (result == VM_PAGER_OK)
- return (0); /* pgo_fault did pmap enter */
- else if (result == VM_PAGER_REFAULT)
- return ERESTART;/* try again! */
- else
- return (EACCES);
- }
/*
* now, if the desired page is not shadowed by the amap and we have
bgpd rde decide all for route-servers
On route-servers at IXPs there is often the wish to not have path hiding
because of output filters. On way of solving this is to use per peer RIBs
but the cost (in memory) is very high. This solves this issue in a
different way but with the same (or similar result).
In bgpd all prefixes/paths are sorted by their preference and by default
the best path is put into the Adj-RIB-Out (after that prefix passed the
output filter). This diff changes this behaviour by falling back to the
next prefix in case the current prefix was filtered. This stops path
hiding and announces the next best route. The cost for doing this is a
bump in CPU usage. On the other hand the memory consumption remains the
same.
The feature can be enabled with 'rde evaluate all' globably or per peer.
It is also to turn it off by using 'rde evaluate default' (in case a peer
does not want this).
--
:wq Claudio
? obj
Index: bgpd.conf.5
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.208
diff -u -p -r1.208 bgpd.conf.5
--- bgpd.conf.5 16 Feb 2021 08:29:16 - 1.208
+++ bgpd.conf.5 16 Feb 2021 09:25:30 -
@@ -268,9 +268,18 @@ daemons, such as
.Xr ospfd 8 .
.Pp
.It Xo
-.Ic rde
-.Ic med
-.Ic compare
+.Ic rde Ic evaluate
+.Pq Ic default Ns | Ns Ic all
+.Xc
+If set to
+.Ar all
+keep evaluating alternative paths in case the selected path is filtered
+out.
+By default if a path is filtered by the output filters then no alternative
+path is sent to this peer.
+.Pp
+.It Xo
+.Ic rde Ic med Ic compare
.Pq Ic always Ns | Ns Ic strict
.Xc
If set to
@@ -1151,6 +1160,20 @@ setting.
.Pp
.It Ic remote-as Ar as-number
Set the AS number of the remote system.
+.Pp
+.It Xo
+.Ic rde Ic evaluate
+.Pq Ic default Ns | Ns Ic all
+.Xc
+If set to
+.Ar all
+keep evaluating alternative paths in case the selected path is filtered
+out.
+By default if a path is filtered by the output filters then no alternative
+path is sent to this peer.
+The default is inherited from the global
+.Ic rde Ic evaluate
+setting.
.Pp
.It Ic rib Ar name
Bind the neighbor to the specified RIB.
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.412
diff -u -p -r1.412 bgpd.h
--- bgpd.h 16 Feb 2021 08:29:16 - 1.412
+++ bgpd.h 16 Feb 2021 09:25:30 -
@@ -65,7 +65,8 @@
#defineBGPD_FLAG_DECISION_ROUTEAGE 0x0100
#defineBGPD_FLAG_DECISION_TRANS_AS 0x0200
#defineBGPD_FLAG_DECISION_MED_ALWAYS 0x0400
-#defineBGPD_FLAG_NO_AS_SET 0x0800
+#defineBGPD_FLAG_DECISION_ALL_PATHS0x0800
+#defineBGPD_FLAG_NO_AS_SET 0x1000
#defineBGPD_LOG_UPDATES0x0001
@@ -408,7 +409,8 @@ struct peer_config {
#define PEERFLAG_TRANS_AS 0x01
#define PEERFLAG_LOG_UPDATES 0x02
-#define PEERFLAG_NO_AS_SET 0x04
+#define PEERFLAG_EVALUATE_ALL 0x04
+#define PEERFLAG_NO_AS_SET 0x08
enum network_type {
NETWORK_DEFAULT,/* from network statements */
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.413
diff -u -p -r1.413 parse.y
--- parse.y 16 Feb 2021 08:29:16 - 1.413
+++ parse.y 16 Feb 2021 09:25:30 -
@@ -200,7 +200,7 @@ typedef struct {
%token AS ROUTERID HOLDTIME YMIN LISTEN ON FIBUPDATE FIBPRIORITY RTABLE
%token NONE UNICAST VPN RD EXPORT EXPORTTRGT IMPORTTRGT DEFAULTROUTE
-%token RDE RIB EVALUATE IGNORE COMPARE RTR PORT
+%token RDE RIB EVALUATE DEFAULT ALL IGNORE COMPARE RTR PORT
%token GROUP NEIGHBOR NETWORK
%token EBGP IBGP
%token LOCALAS REMOTEAS DESCR LOCALADDR MULTIHOP PASSIVE MAXPREFIX RESTART
@@ -791,6 +791,12 @@ conf_main : AS as4number {
}
free($4);
}
+ | RDE EVALUATE ALL {
+ conf->flags |= BGPD_FLAG_DECISION_ALL_PATHS;
+ }
+ | RDE EVALUATE DEFAULT {
+ conf->flags &= ~BGPD_FLAG_DECISION_ALL_PATHS;
+ }
| NEXTHOP QUALIFY VIA STRING{
if (!strcmp($4, "bgp"))
conf->flags |= BGPD_FLAG_NEXTHOP_BGP;
@@ -1727,6 +1733,12 @@ peeropts : REMOTEAS as4number{
else
curpeer->conf.flags &= ~PEERFLAG_NO_AS_SET;
}
+ | RDE EVALUATE ALL {
+ curpeer->conf.flags |= PEERFLAG_EVALUATE_ALL;
+ }
+ | RDE EVALUATE DEFAULT {
+ curpeer->conf.flags &= ~PEERFLAG_EVALUATE_ALL;
+ }
;
restart: /* nada */{ $$ = 0; }
@@ -2850,6 +2862,7 @@ lookup(char *s)
{ "IPv4", IPV4},
Re: rw_enter_diag() vs WITNESS
On 15/02/21(Mon) 16:58, Visa Hankala wrote:
> On Mon, Feb 15, 2021 at 11:37:45AM +0100, Martin Pieuchot wrote:
> > Diagnostic function rw_enter_diag() should be called before
> > WITNESS_CHECKORDER() to have proper locking/debugging information.
> >
> > In the case of 'locking against myself' it is currently impossible
> > to know where the lock has been previously acquired. Diff below fixes
> > that, ok?
>
> Based on this description alone, it is not clear to me what exactly
> gets solved. Doesn't the code reach rw_enter_diag() inside the loop
> when trying to recurse on the rwlock?
It does but before reaching WITNESS_CHECKORDER(). So if a panic is
triggered "show all locks" will point to the previous place where the
same lock has been acquired.
Currently it points to the place triggering the panic which doesn't
help figuring the problem.
> Does this change have implications with (panicstr || db_active)?
Indeed, updated diff below.
Index: kern/kern_rwlock.c
===
RCS file: /cvs/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.47
diff -u -p -r1.47 kern_rwlock.c
--- kern/kern_rwlock.c 8 Feb 2021 08:18:45 - 1.47
+++ kern/kern_rwlock.c 16 Feb 2021 09:04:04 -
@@ -167,6 +167,9 @@ rw_exit_write(struct rwlock *rwl)
static void
rw_enter_diag(struct rwlock *rwl, int flags)
{
+ if (panicstr || db_active)
+ return;
+
switch (flags & RW_OPMASK) {
case RW_WRITE:
case RW_READ:
@@ -237,7 +240,11 @@ rw_enter(struct rwlock *rwl, int flags)
int error, prio;
#ifdef WITNESS
int lop_flags;
+#endif
+
+ rw_enter_diag(rwl, flags);
+#ifdef WITNESS
lop_flags = LOP_NEWORDER;
if (flags & RW_WRITE)
lop_flags |= LOP_EXCLUSIVE;
@@ -270,8 +277,6 @@ retry:
continue;
}
#endif
-
- rw_enter_diag(rwl, flags);
if (flags & RW_NOSLEEP)
return (EBUSY);
