Re: [PATCH] man - apmd(8) reference
Hi, wdaver wrote on Sun, Jun 19, 2022 at 02:09:06PM -0700: > Ah - mangl returns Whoa, i wasn't even aware that "mangl" exists, but it appears it does exist and we even have a port for it: sysutils/mangl. It isn't very happy on my machine, though: schwarze@isnote $ mangl mangl Segmentation fault (core dumped) schwarze@isnote $ mangl man Segmentation fault (core dumped) schwarze@isnote $ mangl apm Segmentation fault (core dumped) schwarze@isnote $ mangl 4 apm Segmentation fault (core dumped) schwarze@isnote $ mangl schwarze@isnote $ It appears mangl is using an embedded copy of mandoc code that is between two and three years old. Consequently, using it is probably a bad idea. That said, adding to what Theo explained, man 4 apm and man -s 4 apm also return the "Alliance ProMotion video driver" manual on all architectures except amd64, arm64, i386, loongson, and macppc, which you can verify with these commands even on amd64: schwarze@isnote $ man -k ^apm\$ apm(4/amd64) - power management interface apm(4/arm64) - power management interface apm(4/i386) - advanced power management device interface apm(4/loongson) - advanced power management device interface apm(4/macppc) - advanced power management device interface apm(4) - Alliance ProMotion video driver apm, zzz, ZZZ(8) - Advanced Power Management control program schwarze@isnote $ man -S foo -s 4 apm | head -n 4 APM(4) Device Drivers ManualAPM(4) NAME apm - Alliance ProMotion video driver Admittedly, it is unfortunate that the Xenocara apm(4) page clashes with the base system apm(4) pages. But i don't think we can do anything about it. The system works as designed: the man(1) command prefers base system pages over Xenocara pages by default, so on the platforms having "advanced power management", you get that, and on the others, you get "Alliance ProMotion". It's not a bug, it's a feature; even if an unfortunate feature in this particular case. > Should I send this as a bug to ports or bugs mailing list? No, i think this thread can be closed as "invalid report". By the way, next time you report a bug, please state, up front, 1. the architecture you are running on 2. the version of OpenBSD you are using 3. and the exact, complete command you are typing. Yours, Ingo
Re: [PATCH] man - apmd(8) reference
3rd time trying to send - this time trying the phone… Ah - mangl returns the "Alliance ProMotion video driver" page for apm(4) when searching and when clicking links. Maybe I need to use another tool... Should I send this as a bug to ports or bugs mailing list? — David Rinehart > On Jun 18, 2022, at 11:56 PM, David Rinehart wrote: > > Ahhh - mangl returns the "Alliance ProMotion video driver" page for > apm(4) when searching and when clicking links. Maybe I need to use > another tool... > > > Should I send this as a bug to ports or bugs mailing list? > > >> On 6/18/22 23:46, Theo Buehler wrote: >>> On Sat, Jun 18, 2022 at 11:37:00PM -0700, David Rinehart wrote: >>> apm(4): apm - Alliance ProMotion video driver >> This reference is about the power management apm kernel driver which >> exists only on amd64, arm64, i386, loongson, and macppc, e.g.: >> >> https://man.openbsd.org/man4/amd64/apm.4
X11 startup scripts: sync ssh key list
The X11 session startup files for xenodm and xinit share the same
snippet which checks if any of the default ssh private key files
exist, and if so, starts ssh-agent and runs ssh-add.
The list of key files is outdated. SSH1 "identity" is gone, and
"id_ecdsa_sk" and "id_ed25519_sk" have been added for FIDO-based
keys. The patch below updates the list and rephrases the shell
code a bit.
I have tested the xenodm Xsession script.
ok?
diff c65268ffd62e66b859aaf782a75cfa2f086925cb /usr/xenocara
blob - 1a1b6ab21ddb00673ddb17d7fb5f855ad21aa461
file + app/xenodm/config/Xsession.in
--- app/xenodm/config/Xsession.in
+++ app/xenodm/config/Xsession.in
@@ -23,18 +23,22 @@ else
fi
# if we have private ssh key(s), start ssh-agent and add the key(s)
-id1=$HOME/.ssh/identity
-id2=$HOME/.ssh/id_dsa
-id3=$HOME/.ssh/id_rsa
-id4=$HOME/.ssh/id_ecdsa
-id5=$HOME/.ssh/id_ed25519
-if [ -z "$SSH_AGENT_PID" ];
+if [ -z "$SSH_AGENT_PID" ] && [ -x /usr/bin/ssh-agent ]
then
- if [ -x /usr/bin/ssh-agent ] && [ -f $id1 -o -f $id2 -o -f $id3 -o -f
$id4 -o -f $id5 ];
- then
- eval `ssh-agent -s`
- ssh-add < /dev/null
- fi
+ for keyfile in \
+ "$HOME/.ssh/id_rsa" \
+ "$HOME/.ssh/id_ecdsa" \
+ "$HOME/.ssh/id_ecdsa_sk" \
+ "$HOME/.ssh/id_ed25519" \
+ "$HOME/.ssh/id_ed25519_sk" \
+ "$HOME/.ssh/id_dsa"
+ do
+ if [ -f "$keyfile" ]; then
+ eval `ssh-agent -s`
+ ssh-add < /dev/null
+ break
+ fi
+ done
fi
do_exit() {
blob - 4c9c3ae8aae50e3fe43f02aa1fb13b72712cbb64
file + app/xinit/xinitrc.cpp
--- app/xinit/xinitrc.cpp
+++ app/xinit/xinitrc.cpp
@@ -41,19 +41,23 @@ if [ -f "$usermodmap" ]; then
fi
XCOMM if we have private ssh key(s), start ssh-agent and add the key(s)
-id1=$HOME/.ssh/identity
-id2=$HOME/.ssh/id_dsa
-id3=$HOME/.ssh/id_rsa
-id4=$HOME/.ssh/id_ecdsa
-id5=$HOME/.ssh/id_ed25519
-if [ -z "$SSH_AGENT_PID" ];
+if [ -z "$SSH_AGENT_PID" ] && [ -x /usr/bin/ssh-agent ]
then
- if [ -x /usr/bin/ssh-agent ] && [ -f $id1 -o -f $id2 -o -f $id3 -o -f
$id4 -o -f $id5 ];
- then
- eval `ssh-agent -s`
- ssh-add < /dev/null
- fi
+ for keyfile in \
+ "$HOME/.ssh/id_rsa" \
+ "$HOME/.ssh/id_ecdsa" \
+ "$HOME/.ssh/id_ecdsa_sk" \
+ "$HOME/.ssh/id_ed25519" \
+ "$HOME/.ssh/id_ed25519_sk" \
+ "$HOME/.ssh/id_dsa"
+ do
+ if [ -f "$keyfile" ]; then
+ eval `ssh-agent -s`
+ ssh-add < /dev/null
+ break
+ fi
+ done
fi
XCOMM start some nice programs
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Fix lock order reversal in nfs_inactive()
On Sun, Jun 19 2022, Visa Hankala wrote: > On Sun, Jun 19, 2022 at 11:05:38AM +0200, Jeremie Courreges-Anglas wrote: >> On Fri, Jun 17 2022, Jeremie Courreges-Anglas wrote: >> > On Thu, Jun 16 2022, Visa Hankala wrote: >> >> nfs_inactive() has a lock order reversal. When it removes the silly >> >> file, it locks the directory vnode while it already holds the lock >> >> of the argument file vnode. This clashes for example with name lookups >> >> where directory vnodes are locked before file vnodes. >> >> >> >> The reversal can cause a deadlock when an NFS client has multiple >> >> processes that create, modify and remove files in the same >> >> NFS directory. >> >> >> >> The following patch makes the silly file removal happen after >> >> nfs_inactive() has released the file vnode lock. This should be safe >> >> because the silly file removal is independent of nfs_inactive()'s >> >> argument vnode. >> >> The diff makes sense to me. Did you spot it reviewing the code, or >> using WITNESS? > > I noticed it by code review. > > WITNESS is somewhat helpless with vnode locks because they can involve > multiple levels of lock nesting. In fact, the order checking between > vnodes has been disabled by initializing the locks with RWL_IS_VNODE. > To fix this, the kernel would have to pass nesting information around > the filesystem code. I see, thanks for confirming. > This particular deadlock can be triggered for example by quickly > writing and removing temporary files in an NFS directory using one > process while another process lists the directory contents repeatedly. > >> >> Could some NFS users test this? >> > >> > I'm running this diff on the riscv64 build cluster, since 1h25mn with no >> > hang. Let's see how it goes. >> >> This run did finish properly yesterday. >> >> > This cluster doesn't use NFS as much as it could (build logs stored >> > locally) but I can try that in the next build. >> >> So I have restarted a build with this diff and dpb(1) logging on an >> NFS-mounted /usr/ports/logs. I get a dpb(1) hang after 1400+ packages >> built. Any other attempt to access the NFS-mounted filesystem results >> in a hang. Let me know if I can extract more data from the system. > > No need this time. Those wait messages give some pointers. Another hang, slightly different excerpt below. >> shannon ~$ grep nfs riscv64/nfs-hang.txt >> 97293 72036 49335 0 30x91 nfs_fsync perl >> 69045 83700 64026 55 30x82 nfs_fsync c++ >> 80365 37354 15104 55 30x100082 nfs_fsync make >> 28876 139812 59322 55 30x100082 nfs_fsync make >> 6160 193238 61541 1000 30x13 nfsnode ksh >> 7535 421732 0 0 3 0x14280 nfsrcvlk nfsio >> 70437 237308 0 0 3 0x14280 nfsrcvlk nfsio >> 97073 406345 0 0 3 0x14200 nfsrcvlk nfsio >> 88487 390804 0 0 3 0x14200 nfsrcvlk nfsio >> 58945 91139 92962 0 30x80 nfsd nfsd >> 75619 357314 92962 0 30x80 nfsd nfsd >> 39027 137228 92962 0 30x80 nfsd nfsd >> 22028 406380 92962 0 30x80 nfsd nfsd >> 92962 11420 1 0 30x80 netconnfsd >> 90467 310188 0 0 3 0x14280 nfsrcvlk update shannon ~$ grep -e nfs riscv64/nfs-hang-2.txt 54340 24206 19639 1000 30x13 nfsnode ksh 43709 63222 55473 55 30x82 nfs_fsync cmake 27658 186996 47176 55 30x100082 nfs_fsync make 38397 435956 77676 55 30x82 nfsrcvlk c++ 37625 117156 96553 0 30x13 nfsnode ssh 96553 293049 2676 0 30x93 nfsrcvlk perl 72080 505789 0 0 3 0x14280 nfsrcvlk nfsio 68474 378836 0 0 3 0x14200 nfsrcvlk nfsio 98412 13475 0 0 3 0x14280 nfsrcvlk nfsio 41393 345501 0 0 3 0x14200 netio nfsio 75129 466227 20392 0 30x80 nfsd nfsd 7762 364728 20392 0 30x80 nfsd nfsd 65425 254516 20392 0 30x80 nfsd nfsd 8062 263249 20392 0 30x80 nfsd nfsd 20392 248868 1 0 30x80 netconnfsd 96499 295545 0 0 3 0x14280 nfsrcvlk update -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
improve unwind memory usage
Some time ago (it has been years actually), Otto instrumented malloc(3)
to see where unwind is using a lot of memory when it's just sitting
there.
One of the remaining areas is struct config_file with its member
outgoing_avail_ports:
if(!(cfg->outgoing_avail_ports = (int*)calloc(65536, sizeof(int
goto error_exit;
That's a 256k just wasted on OpenBSD.
Some time ago I added and upstreamed
--disable-explicit-port-randomisation to unbound(8) so that it doesn't
need to try to find a random outgoing port on its own. On OpenBSD we can
just trust the kernel to do this for us. Apparently I left that struct
behind which is part of the userland machinery in unbound.
Especially in unwind this burns a lot of memory because this is
allocated per libunbound context. Without a config file memory usage
goes down from 10M to 6.7M.
This is a diff to get rid of outgoing_avail_ports in unwind and
unbound. I'll try to upstream it. unbound tests would be very much
appreciated.
Comments, OKs?
diff --git sbin/unwind/libunbound/libunbound/libworker.c
sbin/unwind/libunbound/libunbound/libworker.c
index 11bf5f9db55..941a3d660c8 100644
--- sbin/unwind/libunbound/libunbound/libworker.c
+++ sbin/unwind/libunbound/libunbound/libworker.c
@@ -225,6 +225,7 @@ libworker_setup(struct ub_ctx* ctx, int is_bg, struct
ub_event_base* eb)
if(!w->is_bg || w->is_bg_thread) {
lock_basic_lock(>cfglock);
}
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
numports = cfg_condense_ports(cfg, );
if(numports == 0) {
if(!w->is_bg || w->is_bg_thread) {
@@ -233,6 +234,10 @@ libworker_setup(struct ub_ctx* ctx, int is_bg, struct
ub_event_base* eb)
libworker_delete(w);
return NULL;
}
+#else
+ numports = 1;
+ ports = NULL;
+#endif
w->back = outside_network_create(w->base, cfg->msg_buffer_size,
(size_t)cfg->outgoing_num_ports, cfg->out_ifs,
cfg->num_out_ifs, cfg->do_ip4, cfg->do_ip6,
diff --git sbin/unwind/libunbound/util/config_file.c
sbin/unwind/libunbound/util/config_file.c
index d7bd37a8890..f3713792a25 100644
--- sbin/unwind/libunbound/util/config_file.c
+++ sbin/unwind/libunbound/util/config_file.c
@@ -84,8 +84,10 @@ size_t http2_response_buffer_max = 4 * 1024 * 1024;
/** global config during parsing */
struct config_parser_state* cfg_parser = 0;
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
/** init ports possible for use */
static void init_outgoing_availports(int* array, int num);
+#endif
struct config_file*
config_create(void)
@@ -176,9 +178,11 @@ config_create(void)
cfg->infra_keep_probing = 0;
cfg->delay_close = 0;
cfg->udp_connect = 1;
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
if(!(cfg->outgoing_avail_ports = (int*)calloc(65536, sizeof(int
goto error_exit;
init_outgoing_availports(cfg->outgoing_avail_ports, 65536);
+#endif
if(!(cfg->username = strdup(UB_USERNAME))) goto error_exit;
#ifdef HAVE_CHROOT
if(!(cfg->chrootdir = strdup(CHROOT_DIR))) goto error_exit;
@@ -482,12 +486,14 @@ int config_set_option(struct config_file* cfg, const
char* opt,
} else if(strcmp(opt, "num-threads:") == 0) {
/* not supported, library must have 1 thread in bgworker */
return 0;
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
} else if(strcmp(opt, "outgoing-port-permit:") == 0) {
return cfg_mark_ports(val, 1,
cfg->outgoing_avail_ports, 65536);
} else if(strcmp(opt, "outgoing-port-avoid:") == 0) {
return cfg_mark_ports(val, 0,
cfg->outgoing_avail_ports, 65536);
+#endif
} else if(strcmp(opt, "local-zone:") == 0) {
return cfg_parse_local_zone(cfg, val);
} else if(strcmp(opt, "val-override-date:") == 0) {
@@ -1577,7 +1583,9 @@ config_delete(struct config_file* cfg)
free(cfg->nsid_cfg_str);
free(cfg->nsid);
free(cfg->module_conf);
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
free(cfg->outgoing_avail_ports);
+#endif
config_delstrlist(cfg->caps_whitelist);
config_delstrlist(cfg->private_address);
config_delstrlist(cfg->private_domain);
@@ -1640,6 +1648,7 @@ config_delete(struct config_file* cfg)
free(cfg);
}
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
static void
init_outgoing_availports(int* a, int num)
{
@@ -1668,11 +1677,7 @@ int
cfg_mark_ports(const char* str, int allow, int* avail, int num)
{
char* mid = strchr(str, '-');
-#ifdef DISABLE_EXPLICIT_PORT_RANDOMISATION
- log_warn("Explicit port randomisation disabled, ignoring "
- "outgoing-port-permit and outgoing-port-avoid configuration "
- "options");
-#endif
+
if(!mid) {
int port = atoi(str);
if(port == 0 && strcmp(str, "0") != 0)
pluart(4): change baud rate
Hi,
This allows the pluart baud rate to be changed. There's one potential
pitfall with this change as users will have the wrong baud rate in their
/etc/ttys if not installed after revision 1.11 of dev/ic/pluart.c which
landed today. This will make the serial console unusable until the
expected baud rate in /etc/ttys is changed to 115200.
Comments? OK?
diff --git sys/dev/fdt/pluart_fdt.c sys/dev/fdt/pluart_fdt.c
index 969018eccdc..ac2467bdf47 100644
--- sys/dev/fdt/pluart_fdt.c
+++ sys/dev/fdt/pluart_fdt.c
@@ -27,6 +27,7 @@
#include
#include
+#include
#include
intpluart_fdt_match(struct device *, void *, void *);
@@ -70,8 +71,12 @@ pluart_fdt_attach(struct device *parent, struct device
*self, void *aux)
return;
}
- if (OF_is_compatible(faa->fa_node, "arm,sbsa-uart"))
+ if (OF_is_compatible(faa->fa_node, "arm,sbsa-uart")) {
sc->sc_hwflags |= COM_HW_SBSA;
+ } else {
+ clock_enable_all(faa->fa_node);
+ sc->sc_clkfreq = clock_get_frequency(faa->fa_node, "uartclk");
+ }
periphid = OF_getpropint(faa->fa_node, "arm,primecell-periphid", 0);
if (periphid != 0)
diff --git sys/dev/ic/pluart.c sys/dev/ic/pluart.c
index 40e2b1976fb..aa4301e8fb0 100644
--- sys/dev/ic/pluart.c
+++ sys/dev/ic/pluart.c
@@ -71,9 +71,9 @@
#define UART_ILPR 0x20/* IrDA low-power counter
register */
#define UART_ILPR_ILPDVSR ((x) & 0xf) /* IrDA low-power divisor */
#define UART_IBRD 0x24/* Integer baud rate register */
-#define UART_IBRD_DIVINT ((x) & 0xff)/* Integer baud rate divisor */
+#define UART_IBRD_DIVINT(x)((x) & 0xff)/* Integer baud rate divisor */
#define UART_FBRD 0x28/* Fractional baud rate
register */
-#define UART_FBRD_DIVFRAC ((x) & 0x3f)/* Fractional baud rate divisor
*/
+#define UART_FBRD_DIVFRAC(x) ((x) & 0x3f)/* Fractional baud rate divisor
*/
#define UART_LCR_H 0x2c/* Line control register */
#define UART_LCR_H_BRK (1 << 0)/* Send break */
#define UART_LCR_H_PEN (1 << 1)/* Parity enable */
@@ -338,7 +338,9 @@ pluart_param(struct tty *tp, struct termios *t)
/* lower dtr */
}
- if (ospeed != 0) {
+ if (ospeed != 0 && sc->sc_clkfreq != 0 && tp->t_ospeed != ospeed) {
+ int div, lcr;
+
while (ISSET(tp->t_state, TS_BUSY)) {
++sc->sc_halt;
error = ttysleep(tp, >t_outq,
@@ -349,7 +351,40 @@ pluart_param(struct tty *tp, struct termios *t)
return (error);
}
}
- /* set speed */
+
+ /*
+* Writes to IBRD and FBRD are made effective first when LCR_H
+* is written.
+*/
+ lcr = bus_space_read_4(sc->sc_iot, sc->sc_ioh, UART_LCR_H);
+
+ /* The UART must be disabled while changing the baud rate. */
+ bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_CR, 0);
+
+ /*
+* The baud rate divisor is expressed relative to the UART clock
+* frequency where IBRD represents the quotient using 16 bits
+* and FBRD the remainder using 6 bits. The PL011 specification
+* provides the following formula:
+*
+* uartclk/(16 * baudrate)
+*
+* The formula can be estimated by scaling it with the
+* precision 64 (2^6) and letting the resulting upper 16 bits
+* represents the quotient and the lower 6 bits the remainder:
+*
+* 64 * uartclk/(16 * baudrate) = 4 * uartclk/baudrate
+*/
+ div = 4 * sc->sc_clkfreq / ospeed;
+ bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_IBRD,
+ UART_IBRD_DIVINT(div >> 6));
+ bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_FBRD,
+ UART_FBRD_DIVFRAC(div));
+ /* Commit baud rate change. */
+ bus_space_write_4(sc->sc_iot, sc->sc_ioh, UART_LCR_H, lcr);
+ /* Enable UART. */
+ bus_space_write_4(sc->sc_iot, sc->sc_ioh,
+ UART_CR, UART_CR_UARTEN | UART_CR_TXE | UART_CR_RXE);
}
/* setup fifo */
@@ -594,13 +629,6 @@ pluartopen(dev_t dev, int flag, int mode, struct proc *p)
5 << IMXUART_FCR_RFDIV_SH |
1 << IMXUART_FCR_RXTL_SH);
- bus_space_write_4(iot, ioh, IMXUART_UBIR,
- (pluartdefaultrate / 100) - 1);
-
- /* formula: clk / (rfdiv * 1600) */
- bus_space_write_4(iot, ioh, IMXUART_UBMR,
- (clk_get_rate(sc->sc_clk) * 1000) / 1600);
-
Re: bgpctl use applymask()
On Sun, Jun 19, 2022 at 12:32:39PM +0200, Claudio Jeker wrote: > This diff uses applymask() instead of the IPv4 and IPv6 version. > Makes the code a tiny bit simpler. ok tb > > -- > :wq Claudio > > Index: parser.c > === > RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v > retrieving revision 1.111 > diff -u -p -r1.111 parser.c > --- parser.c 16 Jun 2022 15:34:07 - 1.111 > +++ parser.c 16 Jun 2022 16:51:38 - > @@ -968,17 +968,16 @@ parse_prefix(const char *word, size_t wo > mask = 32; > if (mask > 32) > errx(1, "invalid netmask: too large"); > - inet4applymask(>v4, >v4, mask); > break; > case AID_INET6: > if (mask == -1) > mask = 128; > - inet6applymask(>v6, >v6, mask); > break; > default: > return (0); > } > > + applymask(, , mask); > *prefixlen = mask; > return (1); > } >
bgpctl use applymask()
This diff uses applymask() instead of the IPv4 and IPv6 version. Makes the code a tiny bit simpler. -- :wq Claudio Index: parser.c === RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v retrieving revision 1.111 diff -u -p -r1.111 parser.c --- parser.c16 Jun 2022 15:34:07 - 1.111 +++ parser.c16 Jun 2022 16:51:38 - @@ -968,17 +968,16 @@ parse_prefix(const char *word, size_t wo mask = 32; if (mask > 32) errx(1, "invalid netmask: too large"); - inet4applymask(>v4, >v4, mask); break; case AID_INET6: if (mask == -1) mask = 128; - inet6applymask(>v6, >v6, mask); break; default: return (0); } + applymask(, , mask); *prefixlen = mask; return (1); }
Re: Fix lock order reversal in nfs_inactive()
On Sun, Jun 19, 2022 at 11:05:38AM +0200, Jeremie Courreges-Anglas wrote: > On Fri, Jun 17 2022, Jeremie Courreges-Anglas wrote: > > On Thu, Jun 16 2022, Visa Hankala wrote: > >> nfs_inactive() has a lock order reversal. When it removes the silly > >> file, it locks the directory vnode while it already holds the lock > >> of the argument file vnode. This clashes for example with name lookups > >> where directory vnodes are locked before file vnodes. > >> > >> The reversal can cause a deadlock when an NFS client has multiple > >> processes that create, modify and remove files in the same > >> NFS directory. > >> > >> The following patch makes the silly file removal happen after > >> nfs_inactive() has released the file vnode lock. This should be safe > >> because the silly file removal is independent of nfs_inactive()'s > >> argument vnode. > > The diff makes sense to me. Did you spot it reviewing the code, or > using WITNESS? I noticed it by code review. WITNESS is somewhat helpless with vnode locks because they can involve multiple levels of lock nesting. In fact, the order checking between vnodes has been disabled by initializing the locks with RWL_IS_VNODE. To fix this, the kernel would have to pass nesting information around the filesystem code. This particular deadlock can be triggered for example by quickly writing and removing temporary files in an NFS directory using one process while another process lists the directory contents repeatedly. > >> Could some NFS users test this? > > > > I'm running this diff on the riscv64 build cluster, since 1h25mn with no > > hang. Let's see how it goes. > > This run did finish properly yesterday. > > > This cluster doesn't use NFS as much as it could (build logs stored > > locally) but I can try that in the next build. > > So I have restarted a build with this diff and dpb(1) logging on an > NFS-mounted /usr/ports/logs. I get a dpb(1) hang after 1400+ packages > built. Any other attempt to access the NFS-mounted filesystem results > in a hang. Let me know if I can extract more data from the system. No need this time. Those wait messages give some pointers. > shannon ~$ grep nfs riscv64/nfs-hang.txt > 97293 72036 49335 0 30x91 nfs_fsync perl > 69045 83700 64026 55 30x82 nfs_fsync c++ > 80365 37354 15104 55 30x100082 nfs_fsync make > 28876 139812 59322 55 30x100082 nfs_fsync make > 6160 193238 61541 1000 30x13 nfsnode ksh > 7535 421732 0 0 3 0x14280 nfsrcvlk nfsio > 70437 237308 0 0 3 0x14280 nfsrcvlk nfsio > 97073 406345 0 0 3 0x14200 nfsrcvlk nfsio > 88487 390804 0 0 3 0x14200 nfsrcvlk nfsio > 58945 91139 92962 0 30x80 nfsd nfsd > 75619 357314 92962 0 30x80 nfsd nfsd > 39027 137228 92962 0 30x80 nfsd nfsd > 22028 406380 92962 0 30x80 nfsd nfsd > 92962 11420 1 0 30x80 netconnfsd > 90467 310188 0 0 3 0x14280 nfsrcvlk update
Re: Fix handling of ed addresses consisting only of separators
PING.
I have executed the ed tests (available in bin/ed/test) with this patch
applied and the proposed patch doesn't seem to introduce any new
regressions.
If the patch needs to be revised further, please let me know. The only
thing I can think of right now is that it might make sense to rename the
`recur` variable to `sawseparator` or something along those lines.
Greetings,
Sören
Sören Tempel wrote:
> Hello,
>
> The POSIX specification of ed(1) includes a table in the rationale
> section which (among others) mandates the following address handling
> rules [1]:
>
>Address Addr1 Addr2
>,, $ $
>;; $ $
>
> Unfortunately, OpenBSD does not correctly handle these two example
> addresses. As an example, consider the following `,,p` print command:
>
> printf "a\nfoo\nbar\nbaz\n.\n,,p\nQ\n" | ed
>
> This should only print the last line (as `,,p` should expand to `$,$p`)
> but instead prints all lines with OpenBSD ed. This seems to be a
> regression introduced by Jerome Frgagic in 2017 [2]. Prior to this
> change, `,,p` as well as `;;p` correctly printed the last line. The
> aforementioned change added a recursive invocation of next_addr() which
> does not update the local first variable (causing the second address
> separator to be mistaken for the first). The patch below fixes this by
> tracking recursive invocations of next_addr() via an additional function
> parameter.
>
> See also: https://austingroupbugs.net/view.php?id=1582
>
> [1]:
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ed.html#tag_20_38_18
> [2]:
> https://github.com/openbsd/src/commit/b4c905bbf3932a50011ce3436b1e22acc742cfeb
>
> diff --git bin/ed/main.c bin/ed/main.c
> index 231d021ef19..ca1aeb102b3 100644
> --- bin/ed/main.c
> +++ bin/ed/main.c
> @@ -64,7 +64,7 @@ void signal_hup(int);
> void signal_int(int);
> void handle_winch(int);
>
> -static int next_addr(void);
> +static int next_addr(int);
> static int check_addr_range(int, int);
> static int get_matching_node_addr(regex_t *, int);
> static char *get_filename(int);
> @@ -296,7 +296,7 @@ extract_addr_range(void)
>
> addr_cnt = 0;
> first_addr = second_addr = current_addr;
> - while ((addr = next_addr()) >= 0) {
> + while ((addr = next_addr(0)) >= 0) {
> addr_cnt++;
> first_addr = second_addr;
> second_addr = addr;
> @@ -328,7 +328,7 @@ extract_addr_range(void)
>
> /* next_addr: return the next line address in the command buffer */
> static int
> -next_addr(void)
> +next_addr(int recur)
> {
> char *hd;
> int addr = current_addr;
> @@ -382,11 +382,15 @@ next_addr(void)
> case '%':
> case ',':
> case ';':
> - if (first) {
> + if (first && !recur) {
> ibufp++;
> addr_cnt++;
> second_addr = (c == ';') ? current_addr : 1;
> - if ((addr = next_addr()) < 0)
> +
> + // If the next address is omitted (e.g. "," or
> ";") or
> + // if it is also a separator (e.g. ",," or
> ";;") then
> + // return the last address (as per the omission
> rules).
> + if ((addr = next_addr(1)) < 0)
> addr = addr_last;
> break;
> }
Re: Fix lock order reversal in nfs_inactive()
On Fri, Jun 17 2022, Jeremie Courreges-Anglas wrote: > On Thu, Jun 16 2022, Visa Hankala wrote: >> nfs_inactive() has a lock order reversal. When it removes the silly >> file, it locks the directory vnode while it already holds the lock >> of the argument file vnode. This clashes for example with name lookups >> where directory vnodes are locked before file vnodes. >> >> The reversal can cause a deadlock when an NFS client has multiple >> processes that create, modify and remove files in the same >> NFS directory. >> >> The following patch makes the silly file removal happen after >> nfs_inactive() has released the file vnode lock. This should be safe >> because the silly file removal is independent of nfs_inactive()'s >> argument vnode. The diff makes sense to me. Did you spot it reviewing the code, or using WITNESS? >> Could some NFS users test this? > > I'm running this diff on the riscv64 build cluster, since 1h25mn with no > hang. Let's see how it goes. This run did finish properly yesterday. > This cluster doesn't use NFS as much as it could (build logs stored > locally) but I can try that in the next build. So I have restarted a build with this diff and dpb(1) logging on an NFS-mounted /usr/ports/logs. I get a dpb(1) hang after 1400+ packages built. Any other attempt to access the NFS-mounted filesystem results in a hang. Let me know if I can extract more data from the system. shannon ~$ grep nfs riscv64/nfs-hang.txt 97293 72036 49335 0 30x91 nfs_fsync perl 69045 83700 64026 55 30x82 nfs_fsync c++ 80365 37354 15104 55 30x100082 nfs_fsync make 28876 139812 59322 55 30x100082 nfs_fsync make 6160 193238 61541 1000 30x13 nfsnode ksh 7535 421732 0 0 3 0x14280 nfsrcvlk nfsio 70437 237308 0 0 3 0x14280 nfsrcvlk nfsio 97073 406345 0 0 3 0x14200 nfsrcvlk nfsio 88487 390804 0 0 3 0x14200 nfsrcvlk nfsio 58945 91139 92962 0 30x80 nfsd nfsd 75619 357314 92962 0 30x80 nfsd nfsd 39027 137228 92962 0 30x80 nfsd nfsd 22028 406380 92962 0 30x80 nfsd nfsd 92962 11420 1 0 30x80 netconnfsd 90467 310188 0 0 3 0x14280 nfsrcvlk update -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: pluart(4): hardware console baudrate
On Wed, Jun 15, 2022 at 07:30:09AM +0200, Anton Lindqvist wrote: > pluart(4) does not report the correct baudrate for the hardware console > but instead defaults to 38400. This in turn causes the same baudrate to > end up in /etc/ttys during installation. Note that this is not a problem > as of now since pluart does not support changing the baudrate just yet, > I have another subsequent diff for that. > > Instead, honor and propagate the baudrate given to pluartcnattach() > while attaching the hardware console. Similar to what com(4) already > does. > > Comments? OK? OK visa@
Re: [PATCH] man - apmd(8) reference
On Sat, Jun 18, 2022 at 11:37:00PM -0700, David Rinehart wrote: > apm(4): apm - Alliance ProMotion video driver This reference is about the power management apm kernel driver which exists only on amd64, arm64, i386, loongson, and macppc, e.g.: https://man.openbsd.org/man4/amd64/apm.4
Re: [PATCH] man - apmd(8) reference
apm(4): apm - Alliance ProMotion video driver On 6/18/22 23:34, Theo Buehler wrote: > On Sat, Jun 18, 2022 at 10:14:18PM -0700, David Rinehart wrote: >> apm(4) seems unrelated to apmd(8) > What makes you think so? > > DESCRIPTION > apmd monitors the advanced power management device, apm(4), acting on > signaled events and upon user requests as sent by the apm(8) program.
Re: [PATCH] man - apmd(8) reference
On Sat, Jun 18, 2022 at 10:14:18PM -0700, David Rinehart wrote: > apm(4) seems unrelated to apmd(8) What makes you think so? DESCRIPTION apmd monitors the advanced power management device, apm(4), acting on signaled events and upon user requests as sent by the apm(8) program.
[PATCH] man - apmd(8) reference
apm(4) seems unrelated to apmd(8) Index: usr.sbin/apmd/apmd.8 === RCS file: /cvs/src/usr.sbin/apmd/apmd.8,v retrieving revision 1.55 diff -u -p -u -p -r1.55 apmd.8 --- usr.sbin/apmd/apmd.8 28 May 2022 16:07:54 - 1.55 +++ usr.sbin/apmd/apmd.8 19 Jun 2022 05:01:30 - @@ -191,7 +191,6 @@ socket used for communication with .El .Sh SEE ALSO .Xr syslog 3 , -.Xr apm 4 , .Xr apm 8 , .Xr sysctl 8 .Pp -- David Rinehart
