Re: cat(1): simplify/flatten argument loops
This one reads a lot better to me and even shaves of 2LoC. :-)
OK martijn@
On Mon, 2020-11-30 at 18:28 -0600, Scott Cheloha wrote:
> Hi,
>
> The cook_args() and raw_args() functions in cat(1) are too clever.
> They handle multiple special cases in a single big loop with lots of
> branches. It's been like this since at least 1989:
>
> https://svnweb.freebsd.org/csrg/bin/cat/cat.c?view=markup&pathrev=37179
>
> The goal seems to be to avoid calling cook_buf()/raw_cat() from
> multiple places. I think the result is convoluted. If we isolated
> the special cases and called cook_buf()/raw_cat() from multiple places
> the result would be simpler and flatter.
>
> You can break the cleanup in each function into four steps:
>
> 1. Pull the no-args case out of the loop and handle it first. Now we
> don't need to check if (*argv == NULL) in the body of the loop. One
> fewer assignment to fp/fd, too.
>
> 2. In the loop, isolate the (strcmp(*argv, "-") == 0) special case
> from the normal filename case. Now we don't need to check whether
> we're working with stdin when we clean up at the end of a loop
> iteration. Setup and cleanup are adjacent, no additional branches
> needed.
>
> 3. Pass the file name as an argument to cook_buf() and raw_cat().
> Now we don't need the global 'filename' variable. Obviously
> this means we don't need to assign it a value, either.
>
> 4. Use a for-loop and move argv iteration into the loop header.
> Now we increment argv in a single place in the loop.
>
> Thoughts?
>
> Index: cat.c
> ===
> RCS file: /cvs/src/bin/cat/cat.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 cat.c
> --- cat.c 28 Jun 2019 13:34:58 - 1.27
> +++ cat.c 1 Dec 2020 00:24:20 -
> @@ -51,12 +51,11 @@ extern char *__progname;
>
> int bflag, eflag, nflag, sflag, tflag, vflag;
> int rval;
> -char *filename;
>
> void cook_args(char *argv[]);
> -void cook_buf(FILE *);
> +void cook_buf(FILE *, const char *);
> void raw_args(char *argv[]);
> -void raw_cat(int);
> +void raw_cat(int, const char *);
>
> int
> main(int argc, char *argv[])
> @@ -110,30 +109,29 @@ cook_args(char **argv)
> {
> FILE *fp;
>
> - fp = stdin;
> - filename = "stdin";
> - do {
> - if (*argv) {
> - if (!strcmp(*argv, "-"))
> - fp = stdin;
> - else if ((fp = fopen(*argv, "r")) == NULL) {
> - warn("%s", *argv);
> - rval = 1;
> - ++argv;
> - continue;
> - }
> - filename = *argv++;
> - }
> - cook_buf(fp);
> - if (fp == stdin)
> + if (*argv == NULL) {
> + cook_buf(stdin, "stdin");
> + return;
> + }
> +
> + for (; *argv != NULL; argv++) {
> + if (!strcmp(*argv, "-")) {
> + cook_buf(stdin, "stdin");
> clearerr(fp);
> - else
> - (void)fclose(fp);
> - } while (*argv);
> + continue;
> + }
> + if ((fp = fopen(*argv, "r")) == NULL) {
> + warn("%s", *argv);
> + rval = 1;
> + continue;
> + }
> + cook_buf(fp, *argv);
> + (void)fclose(fp);
> + }
> }
>
> void
> -cook_buf(FILE *fp)
> +cook_buf(FILE *fp, const char *filename)
> {
> int ch, gobble, line, prev;
>
> @@ -200,28 +198,28 @@ raw_args(char **argv)
> {
> int fd;
>
> - fd = fileno(stdin);
> - filename = "stdin";
> - do {
> - if (*argv) {
> - if (!strcmp(*argv, "-"))
> - fd = fileno(stdin);
> - else if ((fd = open(*argv, O_RDONLY, 0)) == -1) {
> - warn("%s", *argv);
> - rval = 1;
> - ++argv;
> - continue;
> - }
> - filename = *argv++;
> + if (*argv == NULL) {
> + raw_cat(fileno(stdin), "stdin");
> + return;
> + }
> +
> + for(; *argv != NULL; argv++) {
> + if (!strcmp(*argv, "-")) {
> + raw_cat(fileno(stdin), "stdin");
> + continue;
> + }
> + if ((fd = open(*argv, O_RDONLY, 0)) == -1) {
> + warn("%s", *argv);
> + rval = 1;
> + continue;
> }
> - raw_cat(fd);
> - if (fd != fileno(stdin))
> - (void)close(f
Re: wireguard + witness
On Mon, Nov 30, 2020 at 11:14:46PM +, Stuart Henderson wrote:
> Thought I'd try a WITNESS kernel to see if that gives any clues about
> what's going on with my APU crashing all over the place (long shot but
> I got bored with trying different older kernels..)
>
> I see these from time to time (one during netstart, and another 4 in
> 15 mins uptime), anyone know if they're important?
this check ("lock_object uninitialized") was recently added in witness.
it means that the rwlock was uninitialized (the witness flag
LO_INITIALIZED isn't present whereas it should) when witness check the
lock.
it could be:
- someone omits to call rw_init() or RWLOCK_INITIALIZER() (possible bug if
memory isn't zero)
- the struct has been zeroed (possible bug too)
> witness: lock_object uninitialized: 0x80bcf0d8
> Starting stack trace...
> witness_checkorder(80bcf0d8,9,0) at witness_checkorder+0xab
> rw_enter_write(80bcf0c8) at rw_enter_write+0x43
> noise_remote_decrypt(80bcea48,978dc3d,0,fd805e22cb7c,10) at
> noise_remote_decrypt+0x135
> wg_decap(8054a000,fd8061835200) at wg_decap+0xda
> wg_decap_worker(8054a000) at wg_decap_worker+0x7a
> taskq_thread(8012d900) at taskq_thread+0x9f
> end trace frame: 0x0, count: 251
> End of stack trace.
from the trace, the sole rw_enter_write() usage in noise_remote_decrypt() is
rw_enter_write(&r->r_keypair_lock)
but I am seeing few rw_init() on r_keypair_lock. I will see if I could
see the source of the problem.
thanks.
--
Sebastien Marie
Re: ACPI diff that needs wide testing
Mark Kettenis writes: > The diff below fixes the way we handle named references in AML > packages. This fixes some bugs but I'd like to make sure that this > doesn't inadvertedly break things. So tests on a wide variety of > machines are welcome. I see a prompt crash: https://photos.app.goo.gl/5NkwAHA6jxrXFzEV7 OCRed: 7b880 size Ox50 previous type ACPI (0xdeaf4151 = 0xdeaf4152) Stopped at db_enter+0x10: : pop %rbp TID PID UID PR FLAGS PFLAGS 0x1 0x200 CPU COMMAND 0K swapper db_enter(10, 8251cf10,282,8, 81887a30, 8251cf18) at db_ enter +0x10 panic(81dd9174, 81dd9174, 3f, 8le23d0e, 80080807b888,9) at panic+0x12a malloc(50,21,9,50,69ffcbe5966771e0, 8010d488) at malloc+0x783 aml_parse(8810d488,69, 8003a810, 8010d488, ee7eae2dea48b7a 4, 8010d488) at aml_parse+0x2a32 aml_parse(8010d488, 54,0,80008818d488, ee?eae2dea 4983ef,80880818d488) at aml_parse+0x3bc aml_eval(80176f88, 808088082108, 74, 18021, 8, 800888176f88) at aml_eval+0x317 aml_parse(80176f88, 74, 7d, 80176f88, ee?eae2dea 48b7a4, 89800017 6f88) at aml_parse+0x2df6 aml_parse(808080176f88,54, 88176f88, 88176f88, ee7eae2dea4983e, f80176f88) at aml_parse+8x3bc aml_eval(0, 80081a08, 74,0,0,0) at aml_eval+0x317 aml_evalnode(88008202c, 80082008,8,0, 8251d648,8808088 8202c) at aml_evalnode +0xb4 aml_evalinteger(8002e400, 80082c88, 81ddbf04,8,8, 8251d6f0) at aml_evalinteger+0x4e acpitz_gettempreading(80257800, 81ddbf04, dad3d43216419857, ff ff8251d73b, 80257800, aac) at acpitz_gettempreading +0x37 acpitz_attach(8002e400, 80257800, 8251d818, 8002e4 08,44b6122d7f8019d2, 8002e480) at acpitz_attach+Oxlea config_attach(80028400, 8210ef70, 82510818, 817576 40,68aa0891922ad23e, 80082c88) at config_attach+8x1f4 end trace frame: 0x8251d950, count: 0 The contents of cp -R /var/db/acpi /tmp && iasl -d /tmp/acpi/* are here: https://blackgnezdo.github.io/hp-elitebook-840.tgz dmesg when not running your patch: OpenBSD 6.8-current (GENERIC.MP) #69: Mon Nov 30 19:53:29 PST 2020 g...@hippy.nest.cx:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 17055608832 (16265MB) avail mem = 16523407360 (15757MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xbbe3f000 (33 entries) bios0: vendor Hewlett-Packard version "L71 Ver. 01.20" date 07/28/2014 bios0: Hewlett-Packard HP EliteBook 840 G1 acpi0 at bios0: ACPI 5.0 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP HPET APIC MCFG TCPA SSDT SSDT FPDT BGRT SSDT SSDT SSDT SSDT SSDT ASF! DMAR acpi0: wakeup devices LANC(S5) EHC1(S3) XHC_(S3) PCIB(S5) NIC_(S5) RP04(S5) WNIC(S5) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz, 1995.70 MHz, 06-45-01 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz, 1995.39 MHz, 06-45-01 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 1, core 0, package 0 cpu2 at mainbus0: apid 2 (application processor) cpu2: Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz, 1995.40 MHz, 06-45-01 cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP
Re: stdio: fclose(3), fopen(3): intended locking hierarchy?
On Mon, Nov 30, 2020 at 6:10 PM Scott Cheloha wrote: > On Sat, Nov 28, 2020 at 01:41:50PM -0800, Philip Guenther wrote: > ... > > After thinking through states more, #4 isn't safe: _fwalk() can't take > > sfp_mutex because it's called from __srefill() which has its FILE locked, > > which would reverse the order: two concurrent calls to __srefill() from > > line-buffered FILEs could have one in _fwalk() blocking on locking the > > other, while the other blocks on the sfp_mutex for _fwalk(). > > This part in __srefill(): > > /* > * Before reading from a line buffered or unbuffered file, > * flush all line buffered output files, per the ANSI C > * standard. > */ > ... > Where in the standard(s) is this behavior required? I'm not even sure > how to look for it. > Pick a version of the C standard and search for "buffered" until you find something like this, which is part of 7.19.3p3 in the draft I'm looking at: <...> When a stream is line buffered, characters are intended to be transmitted to or from the host environment as a block when a new-line character is encountered. Furthermore, characters are intended to be transmitted as a block to the host environment when a buffer is filled, when input is requested on an unbuffered stream, or when input is requested on a line buffered stream that requires the transmission of characters from the host environment. Support for these characteristics is implementation-defined, and may be affected via the setbuf and setvbuf functions. Effectively the same text appears in the POSIX standard in XSH 2.5p6. Basically, you're allowed to stop doing that by instead printing out your cell-phone number so that everyone who wants to complain that their program failed to output its prompt before blocking for input can call and scream at you. :D Philip Guenther
[PATCH] Convert ddb_sysctl to sysctl_bounded_arr
Tested with a bunch of manual sysctl -w's.
OK?
>From 24ae202fd5d39c3c40c029fb878aa15eee33b709 Mon Sep 17 00:00:00 2001
From: Greg Steuck
Date: Mon, 30 Nov 2020 19:42:19 -0800
Subject: [PATCH] Convert ddb_sysctl to sysctl_bounded_arr
---
sys/ddb/db_usrreq.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git sys/ddb/db_usrreq.c sys/ddb/db_usrreq.c
index 546822459ca..259e7b56a8f 100644
--- sys/ddb/db_usrreq.c
+++ sys/ddb/db_usrreq.c
@@ -36,6 +36,14 @@
intdb_log = 1;
intdb_profile; /* Allow dynamic profiling */
+const struct sysctl_bounded_args ddb_vars[] = {
+ { DBCTL_RADIX, &db_radix, 8, 16 },
+ { DBCTL_MAXWIDTH, &db_max_width, 0, INT_MAX },
+ { DBCTL_TABSTOP, &db_tab_stop_width, 1, 16 },
+ { DBCTL_MAXLINE, &db_max_line, 0, INT_MAX },
+ { DBCTL_LOG, &db_log, 0, 1 },
+};
+
int
ddb_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
size_t newlen, struct proc *p)
@@ -47,15 +55,6 @@ ddb_sysctl(int *name, u_int namelen, void *oldp, size_t
*oldlenp, void *newp,
return (ENOTDIR);
switch (name[0]) {
-
- case DBCTL_RADIX:
- return sysctl_int(oldp, oldlenp, newp, newlen, &db_radix);
- case DBCTL_MAXWIDTH:
- return sysctl_int(oldp, oldlenp, newp, newlen, &db_max_width);
- case DBCTL_TABSTOP:
- return sysctl_int(oldp, oldlenp, newp, newlen,
&db_tab_stop_width);
- case DBCTL_MAXLINE:
- return sysctl_int(oldp, oldlenp, newp, newlen, &db_max_line);
case DBCTL_PANIC:
if (securelevel > 0)
return (sysctl_int_lower(oldp, oldlenp, newp, newlen,
@@ -86,8 +85,6 @@ ddb_sysctl(int *name, u_int namelen, void *oldp, size_t
*oldlenp, void *newp,
return (0);
}
break;
- case DBCTL_LOG:
- return (sysctl_int(oldp, oldlenp, newp, newlen, &db_log));
case DBCTL_TRIGGER:
if (newp && db_console) {
struct process *pr = curproc->p_p;
@@ -119,7 +116,8 @@ ddb_sysctl(int *name, u_int namelen, void *oldp, size_t
*oldlenp, void *newp,
break;
#endif /* DDBPROF */
default:
- return (EOPNOTSUPP);
+ return (sysctl_bounded_arr(ddb_vars, nitems(ddb_vars), name,
+ namelen, oldp, oldlenp, newp, newlen));
}
/* NOTREACHED */
}
--
2.29.2
Re: stdio: fclose(3), fopen(3): intended locking hierarchy?
On Sat, Nov 28, 2020 at 01:41:50PM -0800, Philip Guenther wrote:
> On Fri, Nov 27, 2020 at 10:35 PM Philip Guenther wrote:
> ...
>
> > So yeah, maybe it does work to:
> > 1) make __sfp() FLOCKFILE() the allocated FILE before unlocking sfp_mutex
> > 2) update f{,d,mem,un}open() and open_*memstream() to match (1), unlocking
> >in all paths when the FILE is safe to be accessed by _fwalk(), and
> > locking
> >sfp_mutex around the zeroing of _flags.
> > 3) make fclose() and freopen() also lock sfp_mutex around the zeroing of
> > _flags
> >(should add an _frelease() to findfp.c that does this dance for (2) and
> > (3))
> > 4) make _fwalk() take sfp_mutex, and maybe also a FILE* so the setting of
> >__SIGN can be done under the lock?
> >
> 5) decide how/whether to handle adjust the FLOCKFILE placement in the
> > _fwalk()
> >tree: is the testing of the "is line-buffered" flag in lflush() safe
> > without
> >a lock? Mumble...
> >
>
> After thinking through states more, #4 isn't safe: _fwalk() can't take
> sfp_mutex because it's called from __srefill() which has its FILE locked,
> which would reverse the order: two concurrent calls to __srefill() from
> line-buffered FILEs could have one in _fwalk() blocking on locking the
> other, while the other blocks on the sfp_mutex for _fwalk().
This part in __srefill():
/*
* Before reading from a line buffered or unbuffered file,
* flush all line buffered output files, per the ANSI C
* standard.
*/
if (fp->_flags & (__SLBF|__SNBF)) {
/* Ignore this file in _fwalk to avoid potential deadlock. */
fp->_flags |= __SIGN;
(void) _fwalk(lflush);
fp->_flags &= ~__SIGN;
/* Now flush this file without locking it. */
if ((fp->_flags & (__SLBF|__SWR)) == (__SLBF|__SWR))
__sflush(fp);
}
seems to confound all sensible locking hierarchies.
You need to lock the FILE you're trying to refill. Then you need to
check how it is buffered. If it is buffered in a particular way, this
is supposed to trigger a flush on *all other* line-buffered FILEs?
How can you do that without possible deadlock without first yielding
the lock for the FILE in question? And then you've got a race?
Where in the standard(s) is this behavior required? I'm not even sure
how to look for it.
> Hmm, there's currently a loop between two __srefill() calls like that, as
> there's nothing to force visibility of the __SIGN flag between CPUs so they
> could try to lock each other. Grrr.
>
> Time to check other BSDs and see if they have a good solution to this...
I'd say so.
Re: stdio: fclose(3), fopen(3): intended locking hierarchy?
On Fri, Nov 27, 2020 at 10:35:59PM -0800, Philip Guenther wrote:
> On Wed, Nov 25, 2020 at 4:23 PM Scott Cheloha
> wrote:
>
> > In stdio, which lock are you supposed to take first? The global
> > sfp_mutex or the per-FILE lock?
> >
> > In __sfp() we hold sfp_mutex while iterating through the pool (unsure
> > what else to call it) of FILEs. No two threads can modify the pool at
> > the same time:
> >
> ...
>
> > Note that we set _flags to 1 to reserve it for the current thread
> > before leaving sfp_mutex. Note also that we don't take the per-FILE
> > lock before reading each FILE's _flags.
> >
> > Then look at fclose(3):
> >
> ...
>
> > We check if _flags is zero without any lock. I'm unsure if this is
> > safe.
> >
> > However, we then clean up under the FILE's lock and set _flags to zero
> > without sfp_mutex.
> >
> > ... that can't be right.
> >
> > So, what to do? My immediate thought was to export sfp_mutex and
> > enter it before writing _flags (diff attached). But then the global
> > sfp_mutex is "higher" in the locking hierarchy than the per-FILE lock.
> > That doesn't seem quite right to me.
> >
> > We also modify _flags all over stdio without sfp_mutex, so the rule is
> > inconsistent.
> >
> > Another possibility is to take the per-FILE lock when examining each
> > FILE's _flags during __sfp(). That would be costlier, but then the
> > hierarchy would be reversed.
> >
> > Thoughts?
> >
>
> Let's say that we're willing to presume that changing _flags from
> one non-zero value to another non-zero value will never result in
> a zero value being visible either on this CPU or another one. If
> that's not true, then there's more to fix, but let's start with
> that assumption.
Sure.
> Given that, I think the only unsafe item in what you described above
> is the setting of _flags to zero in various places without either
> holding sfp_mutex or using some sort of membar (or atomic op) to
> guarantee all previous changes to the FILE are visible before the
> flags change is visible.
>
> My reasoning would be that if the setting of _flags from non-zero
> to zero was always the last thing visible, then the code scanning
> the list could be sure that a non-zero flags means no one else has
> any pending writes to the FILE and it can be allocated. __sfp()'s
> setting _flags to 1 to mark it as allocated is made visible to other
> threads when it unlocks sfp_mutex.
>
> ...but we don't have those membars/atomic-ops, so it's not currently
> guaranteed that __sfp() can't allocate a FILE which is still being
> updated by a thread that's releasing it. ;(
We can't use the stuff in sys/atomic.h in userspace?
> If strewing membars makes people twitchy (my eye twitches some),
> then yeah, your proposal to take sfp_mutex when zeroing _file is
> te alternative. Regarding the hierarchy concern, see below.
Ack.
> None of this fixes _fwalk(), which can invoke the callback on
> partially created FILEs, even if it were to grab sfp_mutex. I can
> imagine a couple directions for fixing that, from setting __SIGN
> on not-yet-completed FILEs and clearing it at the end, to full-blown
> having __sfp() return a locked FILE and making _fwalk() lock each
> FILE before invoking the callback. Note that of the three callbacks
> passed to _fwalk(), two end up locking the FILE anyway, so maybe
> this is the right direction anyway.
>
> So, the lock hierarchy is then...interesting:
>
> * if you hold sfp_mutex, you can FLOCKFILE a FILE iff _flags == 0
> * if _flags != 0, you must lock sfp_mutex before zeroing it and
>FUNLOCKFILE and never touch the FILE again before unlocking
>sfp_mutex.
>
> Given the assumption at top, I believe that's safe+correct.
>
> The problem case for _fwalk() is _cleanup(), which currently and
> explicitly 'cheats' by failing to lock FILE...but I suspect that's
> a hold-over from when abort() called atexit() handlers, as it's
> supposed to be async-signal-safe and therefore can't take locks.
> abort() no longer does that: POSIX withdrew it because, well, it
> can't be done safely with an async-signal-safe abort() without
> making lots of stdio functions block all signals, which would lead
> to torches and pitchforks.
Ahhh, that makes sense.
> This is presumably _also_ why _fwalk() doesn't lock sfp_mutex when it
> 'obviously' should, so that's fixable too! Woot!
>
> So yeah, maybe it does work to:
>
> 1) make __sfp() FLOCKFILE() the allocated FILE before unlocking sfp_mutex
>
> 2) update f{,d,mem,un}open() and open_*memstream() to match (1), unlocking
>in all paths when the FILE is safe to be accessed by _fwalk(), and
>locking sfp_mutex around the zeroing of _flags.
>
> 3) make fclose() and freopen() also lock sfp_mutex around the zeroing of
>_flags (should add an _frelease() to findfp.c that does this dance for
>(2) and (3))
>
> 4) make _fwalk() take sfp_mutex, and maybe also a FILE* so the setting of
>__SIGN can be done under the lock?
>
> 5) decide how/w
Re: an(4): tsleep(9) -> tsleep_nsec(9)
On Thu, Nov 26, 2020 at 08:25:48PM +1100, Jonathan Gray wrote:
> On Tue, Nov 24, 2020 at 07:20:46PM -0600, Scott Cheloha wrote:
> > Hi,
> >
> > Both kettenis@ and mpi@ have mentioned in private that my proposed
> > changes to tsleep_nsec(9) etc. would be nicer if we could just get rid
> > of tsleep(9) etc. entirely.
> >
> > This is difficult, but I'll try.
> >
> > Worst case, we thin out the remaining callers. There are not many
> > left.
> >
> > --
> >
> > So, an(4) is one such caller.
> >
> > In an_wait() we spin for (3 * hz) ticks waiting for CSR_WRITE_2 to
> > return the AN_EV_CMD flag. There is no code handling a case where
> > this fails to happen.
> >
> > What we do in practice is very nearly equivalent to spinning for 3
> > seconds waiting for CSR_WRITE_2 to return the AN_EV_CMD flag, so I
> > have converted it to use tsleep_nsec(9).
> >
> > This compiles on amd64 but I can't test it.
> >
> > Thoughts? ok?
>
> I don't see why the upper bound would have to be so precise.
>
> Why not just
>
> for (i = 0; i < 3000; i += 100) {
> if (CSR_READ_2(sc, AN_EVENT_STAT) & AN_EV_CMD)
> break;
> tsleep_nsec(sc, PWAIT, "anatch", MSEC_TO_NSEC(100));
> }
I was just trying to imitate the current behavior as closely as
possible.
If you're fine fudging it like that then I'm fine with it. Just
beware that that tsleep_nsec(9) can and will oversleep by up to 1/hz
seconds.
Did you intend to increase the poll interval from 10ms to 100ms or is
that a typo?
Re: [PATCH] Fix a bug where GDB could not display symbols
I fixed the patch a bit.
The bt command does not work correctly with the core created when
calling the ABORT(3) as follows:
$ cat main.c
#include
#include
#include
int
main(int argc, char *argv[])
{
printf("argc = %d\n", argc);
abort();
return (0);
}
$ cc -static main.c
$ ./a.out
argc = 1
Abort trap (core dumped)
$ /usr/bin/gdb a.out a.out.core
GNU gdb 6.3
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and
you are
welcome to change it and/or distribute copies of it under certain
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for
details.
This GDB was configured as "amd64-unknown-openbsd6.8"...
Core was generated by `a.out'.
Program terminated with signal 6, Aborted.
#0 0x05c6e0400c1a in ?? ()
(gdb) bt
#0 0x05c6e0400c1a in ?? ()
#1 0x05c6e0400bbe in ?? ()
#2 0x00300010 in ?? ()
#3 0x7f7beec0 in ?? ()
#4 0xffdbede0 in ?? ()
#5 0x7f7bef48 in ?? ()
#6 0x7f7beee0 in ?? ()
#7 0x05c6e03ecf5f in ?? ()
#8 0x0007 in ?? ()
#9 0x0009fbe83b4e in ?? ()
#10 0x7f7bef48 in ?? ()
#11 0x0001 in ?? ()
#12 0x7f7bef30 in ?? ()
#13 0x05c6e03ecd23 in ?? ()
#14 0x05c6e0406078 in ?? ()
#15 0x7f7bef58 in ?? ()
#16 0x0001 in ?? ()
#17 0x in ?? ()
(gdb) quit
I applied the new patch to /usr/obj/gnu/usr.bin/binutils/gdb/gdb.
$ /usr/obj/gnu/usr.bin/binutils/gdb/gdb a.out a.out.core
GNU gdb 6.3
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and
you are
welcome to change it and/or distribute copies of it under certain
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for
details.
This GDB was configured as "amd64-unknown-openbsd6.8"...
Core was generated by `a.out'.
Program terminated with signal 6, Aborted.
#0 thrkill () at /tmp/-:3
3 /tmp/-: No such file or directory.
in /tmp/-
(gdb) bt
#0 thrkill () at /tmp/-:3
#1 0x05c6e0400bbe in _libc_abort () at
/usr/src/lib/libc/stdlib/abort.c:51
#2 0x05c6e03ecf5f in main ()
Current language: auto; currently asm
(gdb) quit
$
I added exec_set_section_offsets() after do_cleanups() in the previous
patch.
ok? comment?
Index: solib-svr4.c
===
RCS file: /cvs/src/gnu/usr.bin/binutils/gdb/solib-svr4.c,v
retrieving revision 1.2
diff -u -p -r1.2 solib-svr4.c
--- solib-svr4.c11 Nov 2008 22:57:48 - 1.2
+++ solib-svr4.c30 Nov 2020 23:01:42 -
@@ -619,7 +619,41 @@ svr4_current_sos (void)
/* If we can't find the dynamic linker's base structure, this
must not be a dynamically linked executable. Hmm. */
if (! debug_base)
- return 0;
+ {
+ if (exec_bfd != NULL &&
+ bfd_get_section_by_name (exec_bfd, ".interp") == NULL &&
+ (bfd_get_file_flags (exec_bfd) & DYNAMIC) != 0 &&
+ bfd_get_start_address (exec_bfd) != entry_point_address ())
+ {
+ /* this is relocatable static link.
+cf. svr4_relocate_main_executable() */
+ struct cleanup *old_chain;
+ struct section_offsets *new_offsets;
+ int i, changed;
+ CORE_ADDR displacement;
+
+ displacement = entry_point_address () - bfd_get_start_address
(exec_bfd);
+ changed = 0;
+
+ new_offsets = xcalloc (symfile_objfile->num_sections,
+sizeof (struct section_offsets));
+ old_chain = make_cleanup (xfree, new_offsets);
+
+ for (i = 0; i < symfile_objfile->num_sections; i++)
+ {
+ if (displacement != ANOFFSET
(symfile_objfile->section_offsets, i))
+ changed = 1;
+ new_offsets->offsets[i] = displacement;
+ }
+
+ if (changed)
+ objfile_relocate (symfile_objfile, new_offsets);
+
+ do_cleanups (old_chain);
+ exec_set_section_offsets(displacement, displacement,
displacement);
+ }
+ return 0;
+ }
}
/* Walk the inferior's link map list, and build our list of
--
ASOU Masato
From: Masato Asou
Date: Thu, 08 Oct 2020 14:07:15 +0900 (JST)
> I refferd to the core of static linked in GDB. However, the backtrace
> command did not display the symbols correctly.
>
> $ cat main.c
> #include
>
> void
> sub2(int argc, char *argv[])
> {
> int i;
> for (int i = 0; i <= argc; i++)
> argv[i][0] = '\0';
> }
>
> void
> sub1(int argc, char *argv[])
> {
> sub2(argc, argv);
> }
>
> int
> main(int argc, char *argv[])
> {
> sub1(argc, argv);
> return (0);
> }
> $
cat(1): simplify/flatten argument loops
Hi,
The cook_args() and raw_args() functions in cat(1) are too clever.
They handle multiple special cases in a single big loop with lots of
branches. It's been like this since at least 1989:
https://svnweb.freebsd.org/csrg/bin/cat/cat.c?view=markup&pathrev=37179
The goal seems to be to avoid calling cook_buf()/raw_cat() from
multiple places. I think the result is convoluted. If we isolated
the special cases and called cook_buf()/raw_cat() from multiple places
the result would be simpler and flatter.
You can break the cleanup in each function into four steps:
1. Pull the no-args case out of the loop and handle it first. Now we
don't need to check if (*argv == NULL) in the body of the loop. One
fewer assignment to fp/fd, too.
2. In the loop, isolate the (strcmp(*argv, "-") == 0) special case
from the normal filename case. Now we don't need to check whether
we're working with stdin when we clean up at the end of a loop
iteration. Setup and cleanup are adjacent, no additional branches
needed.
3. Pass the file name as an argument to cook_buf() and raw_cat().
Now we don't need the global 'filename' variable. Obviously
this means we don't need to assign it a value, either.
4. Use a for-loop and move argv iteration into the loop header.
Now we increment argv in a single place in the loop.
Thoughts?
Index: cat.c
===
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.27
diff -u -p -r1.27 cat.c
--- cat.c 28 Jun 2019 13:34:58 - 1.27
+++ cat.c 1 Dec 2020 00:24:20 -
@@ -51,12 +51,11 @@ extern char *__progname;
int bflag, eflag, nflag, sflag, tflag, vflag;
int rval;
-char *filename;
void cook_args(char *argv[]);
-void cook_buf(FILE *);
+void cook_buf(FILE *, const char *);
void raw_args(char *argv[]);
-void raw_cat(int);
+void raw_cat(int, const char *);
int
main(int argc, char *argv[])
@@ -110,30 +109,29 @@ cook_args(char **argv)
{
FILE *fp;
- fp = stdin;
- filename = "stdin";
- do {
- if (*argv) {
- if (!strcmp(*argv, "-"))
- fp = stdin;
- else if ((fp = fopen(*argv, "r")) == NULL) {
- warn("%s", *argv);
- rval = 1;
- ++argv;
- continue;
- }
- filename = *argv++;
- }
- cook_buf(fp);
- if (fp == stdin)
+ if (*argv == NULL) {
+ cook_buf(stdin, "stdin");
+ return;
+ }
+
+ for (; *argv != NULL; argv++) {
+ if (!strcmp(*argv, "-")) {
+ cook_buf(stdin, "stdin");
clearerr(fp);
- else
- (void)fclose(fp);
- } while (*argv);
+ continue;
+ }
+ if ((fp = fopen(*argv, "r")) == NULL) {
+ warn("%s", *argv);
+ rval = 1;
+ continue;
+ }
+ cook_buf(fp, *argv);
+ (void)fclose(fp);
+ }
}
void
-cook_buf(FILE *fp)
+cook_buf(FILE *fp, const char *filename)
{
int ch, gobble, line, prev;
@@ -200,28 +198,28 @@ raw_args(char **argv)
{
int fd;
- fd = fileno(stdin);
- filename = "stdin";
- do {
- if (*argv) {
- if (!strcmp(*argv, "-"))
- fd = fileno(stdin);
- else if ((fd = open(*argv, O_RDONLY, 0)) == -1) {
- warn("%s", *argv);
- rval = 1;
- ++argv;
- continue;
- }
- filename = *argv++;
+ if (*argv == NULL) {
+ raw_cat(fileno(stdin), "stdin");
+ return;
+ }
+
+ for(; *argv != NULL; argv++) {
+ if (!strcmp(*argv, "-")) {
+ raw_cat(fileno(stdin), "stdin");
+ continue;
+ }
+ if ((fd = open(*argv, O_RDONLY, 0)) == -1) {
+ warn("%s", *argv);
+ rval = 1;
+ continue;
}
- raw_cat(fd);
- if (fd != fileno(stdin))
- (void)close(fd);
- } while (*argv);
+ raw_cat(fd, *argv);
+ (void)close(fd);
+ }
}
void
-raw_cat(int rfd)
+raw_cat(int rfd, const char *filename)
{
int wfd;
ssize_t nr, nw, off;
wireguard + witness
Thought I'd try a WITNESS kernel to see if that gives any clues about what's going on with my APU crashing all over the place (long shot but I got bored with trying different older kernels..) I see these from time to time (one during netstart, and another 4 in 15 mins uptime), anyone know if they're important? witness: lock_object uninitialized: 0x80bcf0d8 Starting stack trace... witness_checkorder(80bcf0d8,9,0) at witness_checkorder+0xab rw_enter_write(80bcf0c8) at rw_enter_write+0x43 noise_remote_decrypt(80bcea48,978dc3d,0,fd805e22cb7c,10) at noise_remote_decrypt+0x135 wg_decap(8054a000,fd8061835200) at wg_decap+0xda wg_decap_worker(8054a000) at wg_decap_worker+0x7a taskq_thread(8012d900) at taskq_thread+0x9f end trace frame: 0x0, count: 251 End of stack trace.
ACPI diff that needs wide testing
The diff below fixes the way we handle named references in AML
packages. This fixes some bugs but I'd like to make sure that this
doesn't inadvertedly break things. So tests on a wide variety of
machines are welcome.
Index: dev/acpi/acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.391
diff -u -p -r1.391 acpi.c
--- dev/acpi/acpi.c 27 Aug 2020 01:08:55 - 1.391
+++ dev/acpi/acpi.c 30 Nov 2020 21:00:23 -
@@ -2980,21 +2980,24 @@ acpi_getprop(struct aml_node *node, cons
/* Check properties. */
for (i = 0; i < dsd.v_package[1]->length; i++) {
struct aml_value *res = dsd.v_package[1]->v_package[i];
+ struct aml_value *val;
int len;
if (res->type != AML_OBJTYPE_PACKAGE || res->length != 2 ||
res->v_package[0]->type != AML_OBJTYPE_STRING)
continue;
- len = res->v_package[1]->length;
- switch (res->v_package[1]->type) {
+ val = res->v_package[1];
+ if (val->type == AML_OBJTYPE_OBJREF)
+ val = val->v_objref.ref;
+
+ len = val->length;
+ switch (val->type) {
case AML_OBJTYPE_BUFFER:
- memcpy(buf, res->v_package[1]->v_buffer,
- min(len, buflen));
+ memcpy(buf, val->v_buffer, min(len, buflen));
return len;
case AML_OBJTYPE_STRING:
- memcpy(buf, res->v_package[1]->v_string,
- min(len, buflen));
+ memcpy(buf, val->v_string, min(len, buflen));
return len;
}
}
@@ -3031,14 +3034,22 @@ acpi_getpropint(struct aml_node *node, c
/* Check properties. */
for (i = 0; i < dsd.v_package[1]->length; i++) {
struct aml_value *res = dsd.v_package[1]->v_package[i];
+ struct aml_value *val;
if (res->type != AML_OBJTYPE_PACKAGE || res->length != 2 ||
- res->v_package[0]->type != AML_OBJTYPE_STRING ||
- res->v_package[1]->type != AML_OBJTYPE_INTEGER)
+ res->v_package[0]->type != AML_OBJTYPE_STRING)
+ continue;
+
+ val = res->v_package[1];
+ if (val->type == AML_OBJTYPE_OBJREF)
+ val = val->v_objref.ref;
+
+ if (val->type != AML_OBJTYPE_INTEGER)
continue;
- if (strcmp(res->v_package[0]->v_string, prop) == 0)
- return res->v_package[1]->v_integer;
+ if (strcmp(res->v_package[0]->v_string, prop) == 0 &&
+ val->type == AML_OBJTYPE_INTEGER)
+ return val->v_integer;
}
return defval;
@@ -3130,7 +3141,7 @@ const char *acpi_isa_hids[] = {
void
acpi_attach_deps(struct acpi_softc *sc, struct aml_node *node)
{
- struct aml_value res;
+ struct aml_value res, *val;
struct aml_node *dep;
int i;
@@ -3141,9 +3152,14 @@ acpi_attach_deps(struct acpi_softc *sc,
return;
for (i = 0; i < res.length; i++) {
- if (res.v_package[i]->type != AML_OBJTYPE_STRING)
+ val = res.v_package[i];
+ if (val->type == AML_OBJTYPE_OBJREF)
+ val = val->v_objref.ref;
+ if (val->type != AML_OBJTYPE_DEVICE) {
+ printf("%s: type %d\n", __func__, val->type);
continue;
- dep = aml_searchrel(node, res.v_package[i]->v_string);
+ }
+ dep = val->node;
if (dep == NULL || dep->attached)
continue;
dep = aml_searchname(dep, "_HID");
Index: dev/acpi/acpiprt.c
===
RCS file: /cvs/src/sys/dev/acpi/acpiprt.c,v
retrieving revision 1.49
diff -u -p -r1.49 acpiprt.c
--- dev/acpi/acpiprt.c 11 Apr 2020 11:01:18 - 1.49
+++ dev/acpi/acpiprt.c 30 Nov 2020 21:00:23 -
@@ -272,14 +272,6 @@ acpiprt_prt_add(struct acpiprt_softc *sc
}
pp = v->v_package[2];
- if (pp->type == AML_OBJTYPE_STRING) {
- node = aml_searchrel(sc->sc_devnode, pp->v_string);
- if (node == NULL) {
- printf("Invalid device\n");
- return;
- }
- pp = node->value;
- }
if (pp->type == AML_OBJTYPE_NAMEREF) {
node = aml_searchrel(sc->sc_devnode, pp->v_nameref);
if (node == NULL) {
Index: dev/acpi/acpipwrres.c
===
RCS file: /cvs/src/sys/dev/acpi/acpipwrres.c,
Re: Prevent race in single_thread_set()
On 04/11/20(Wed) 11:19, Martin Pieuchot wrote:
> Here's a 3rd approach to solve the TOCTOU race in single_thread_set().
> The issue being that the lock serializing access to `ps_single' is not
> held when calling single_thread_check().
>
> The approach below is controversial because it extends the scope of the
> SCHED_LOCK(). On the other hand, the two others approaches that both
> add a new lock to avoid this race ignore the fact that accesses to
> `ps_single' are currently not clearly serialized w/o KERNEL_LOCK().
>
> So the diff below improves the situation in that regard and do not add
> more complexity due to the use of multiple locks. After having looked
> for a way to split the SCHED_LOCK() I believe this is the simplest
> approach.
>
> I deliberately used a *_locked() function to avoid grabbing the lock
> recursively as I'm trying to get rid of the recursion, see the other
> thread on tech@.
>
> That said the uses of `ps_single' in ptrace_ctrl() are not covered by
> this diff and I'd be glad to hear some comments about them. This is
> fine as long as all the code using `ps_single' runs under KERNEL_LOCK()
> but since we're trying to get the single_thread_* API out of it, this
> need to be addressed.
>
> Note that this diff introduces a helper for initializing ps_single*
> values in order to keep all the accesses of those fields in the same
> file.
Anyone? With this only the `ps_threads' iteration must receive some
love in order to take the single_thread_* API out of the KERNEL_LOCK().
For that I just sent a SMR_TAILQ conversion diff.
Combined with the diff to remove the recursive attribute of the
SCHED_LOCK() we're ready to split it into multiple mutexes.
Does that make any sense? Comments? Oks?
> Index: kern/kern_fork.c
> ===
> RCS file: /cvs/src/sys/kern/kern_fork.c,v
> retrieving revision 1.226
> diff -u -p -r1.226 kern_fork.c
> --- kern/kern_fork.c 25 Oct 2020 01:55:18 - 1.226
> +++ kern/kern_fork.c 4 Nov 2020 12:52:54 -
> @@ -563,10 +563,7 @@ thread_fork(struct proc *curp, void *sta
>* if somebody else wants to take us to single threaded mode,
>* count ourselves in.
>*/
> - if (pr->ps_single) {
> - atomic_inc_int(&pr->ps_singlecount);
> - atomic_setbits_int(&p->p_flag, P_SUSPSINGLE);
> - }
> + single_thread_init(p);
>
> /*
>* Return tid to parent thread and copy it out to userspace
> Index: kern/kern_sig.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.263
> diff -u -p -r1.263 kern_sig.c
> --- kern/kern_sig.c 16 Sep 2020 13:50:42 - 1.263
> +++ kern/kern_sig.c 4 Nov 2020 12:38:35 -
> @@ -1932,11 +1932,27 @@ userret(struct proc *p)
> p->p_cpu->ci_schedstate.spc_curpriority = p->p_usrpri;
> }
>
> +void
> +single_thread_init(struct proc *p)
> +{
> + struct process *pr = p->p_p;
> + int s;
> +
> + SCHED_LOCK(s);
> + if (pr->ps_single) {
> + atomic_inc_int(&pr->ps_singlecount);
> + atomic_setbits_int(&p->p_flag, P_SUSPSINGLE);
> + }
> + SCHED_UNLOCK(s);
> +}
> +
> int
> -single_thread_check(struct proc *p, int deep)
> +_single_thread_check_locked(struct proc *p, int deep)
> {
> struct process *pr = p->p_p;
>
> + SCHED_ASSERT_LOCKED();
> +
> if (pr->ps_single != NULL && pr->ps_single != p) {
> do {
> int s;
> @@ -1949,14 +1965,12 @@ single_thread_check(struct proc *p, int
> return (EINTR);
> }
>
> - SCHED_LOCK(s);
> - if (pr->ps_single == NULL) {
> - SCHED_UNLOCK(s);
> + if (pr->ps_single == NULL)
> continue;
> - }
>
> if (atomic_dec_int_nv(&pr->ps_singlecount) == 0)
> wakeup(&pr->ps_singlecount);
> +
> if (pr->ps_flags & PS_SINGLEEXIT) {
> SCHED_UNLOCK(s);
> KERNEL_LOCK();
> @@ -1967,13 +1981,24 @@ single_thread_check(struct proc *p, int
> /* not exiting and don't need to unwind, so suspend */
> p->p_stat = SSTOP;
> mi_switch();
> - SCHED_UNLOCK(s);
> } while (pr->ps_single != NULL);
> }
>
> return (0);
> }
>
> +int
> +single_thread_check(struct proc *p, int deep)
> +{
> + int s, error;
> +
> + SCHED_LOCK(s);
> + error = _single_thread_check_locked(p, deep);
> + SCHED_UNLOCK(s);
> +
> + return error;
> +}
> +
> /*
> * Stop other threads in the process. The mode controls how and
> * where the other threads should stop:
> @@ -1995,8 +2020,12 @@ si
Use SMR_TAILQ for `ps_threads'
Every multi-threaded process keeps a list of threads in `ps_threads'.
This list is iterated in interrupt and process context which makes it
complicated to protect it with a rwlock.
One of the places where such iteration is done is inside the tsleep(9)
routines, directly in single_thread_check() or via CURSIG(). In order
to take this code path out of the KERNEL_LOCK(), claudio@ proposed to
use SMR_TAILQ. This has the advantage of not introducing lock
dependencies and allow us to address every iteration one-by-one.
Diff below is a first step into this direction, it replaces the existing
TAILQ_* macros by the locked version of SMR_TAILQ*. This is mostly lifted
from claudio@'s diff and should not introduce any side effect.
ok?
diff --git lib/libkvm/kvm_proc2.c lib/libkvm/kvm_proc2.c
index 96f7dc91b92..1f4f9b914bb 100644
--- lib/libkvm/kvm_proc2.c
+++ lib/libkvm/kvm_proc2.c
@@ -341,8 +341,9 @@ kvm_proclist(kvm_t *kd, int op, int arg, struct process *pr,
kp.p_pctcpu = 0;
kp.p_stat = (process.ps_flags & PS_ZOMBIE) ? SDEAD :
SIDL;
- for (p = TAILQ_FIRST(&process.ps_threads); p != NULL;
- p = TAILQ_NEXT(&proc, p_thr_link)) {
+ for (p = SMR_TAILQ_FIRST_LOCKED(&process.ps_threads);
+ p != NULL;
+ p = SMR_TAILQ_NEXT_LOCKED(&proc, p_thr_link)) {
if (KREAD(kd, (u_long)p, &proc)) {
_kvm_err(kd, kd->program,
"can't read proc at %lx",
@@ -376,8 +377,8 @@ kvm_proclist(kvm_t *kd, int op, int arg, struct process *pr,
if (!dothreads)
continue;
- for (p = TAILQ_FIRST(&process.ps_threads); p != NULL;
- p = TAILQ_NEXT(&proc, p_thr_link)) {
+ for (p = SMR_TAILQ_FIRST_LOCKED(&process.ps_threads); p != NULL;
+ p = SMR_TAILQ_NEXT_LOCKED(&proc, p_thr_link)) {
if (KREAD(kd, (u_long)p, &proc)) {
_kvm_err(kd, kd->program,
"can't read proc at %lx",
diff --git sys/kern/exec_elf.c sys/kern/exec_elf.c
index 5e455208663..575273b306c 100644
--- sys/kern/exec_elf.c
+++ sys/kern/exec_elf.c
@@ -85,6 +85,7 @@
#include
#include
#include
+#include
#include
@@ -1360,7 +1361,7 @@ coredump_notes_elf(struct proc *p, void *iocookie, size_t
*sizep)
* threads in the process have been stopped and the list can't
* change.
*/
- TAILQ_FOREACH(q, &pr->ps_threads, p_thr_link) {
+ SMR_TAILQ_FOREACH_LOCKED(q, &pr->ps_threads, p_thr_link) {
if (q == p) /* we've taken care of this thread */
continue;
error = coredump_note_elf(q, iocookie, ¬esize);
diff --git sys/kern/init_main.c sys/kern/init_main.c
index fed6be19435..2b657ffe328 100644
--- sys/kern/init_main.c
+++ sys/kern/init_main.c
@@ -519,7 +519,7 @@ main(void *framep)
*/
LIST_FOREACH(pr, &allprocess, ps_list) {
nanouptime(&pr->ps_start);
- TAILQ_FOREACH(p, &pr->ps_threads, p_thr_link) {
+ SMR_TAILQ_FOREACH_LOCKED(p, &pr->ps_threads, p_thr_link) {
nanouptime(&p->p_cpu->ci_schedstate.spc_runtime);
timespecclear(&p->p_rtime);
}
diff --git sys/kern/kern_exit.c sys/kern/kern_exit.c
index a20775419e3..ffc0158954c 100644
--- sys/kern/kern_exit.c
+++ sys/kern/kern_exit.c
@@ -63,6 +63,7 @@
#ifdef SYSVSEM
#include
#endif
+#include
#include
#include
@@ -161,7 +162,7 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
}
/* unlink ourselves from the active threads */
- TAILQ_REMOVE(&pr->ps_threads, p, p_thr_link);
+ SMR_TAILQ_REMOVE_LOCKED(&pr->ps_threads, p, p_thr_link);
if ((p->p_flag & P_THREAD) == 0) {
/* main thread gotta wait because it has the pid, et al */
while (pr->ps_refcnt > 1)
@@ -724,7 +725,7 @@ process_zap(struct process *pr)
if (pr->ps_ptstat != NULL)
free(pr->ps_ptstat, M_SUBPROC, sizeof(*pr->ps_ptstat));
pool_put(&rusage_pool, pr->ps_ru);
- KASSERT(TAILQ_EMPTY(&pr->ps_threads));
+ KASSERT(SMR_TAILQ_EMPTY_LOCKED(&pr->ps_threads));
lim_free(pr->ps_limit);
crfree(pr->ps_ucred);
pool_put(&process_pool, pr);
diff --git sys/kern/kern_fork.c sys/kern/kern_fork.c
index 9fb239bc8b4..e1cb587b2b8 100644
--- sys/kern/kern_fork.c
+++ sys/kern/kern_fork.c
@@ -52,6 +52,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -179,8 +180,8 @@ process_initialize(struct process *pr, struct proc *p)
{
/* initialize the thread links */
pr->ps_mainproc = p;
- TAILQ_INIT(&p
Re: Switch select(2) to kqueue-based implementation
On 30/11/20(Mon) 17:06, Visa Hankala wrote:
> On Mon, Nov 30, 2020 at 01:28:14PM -0300, Martin Pieuchot wrote:
> > I plan to commit this in 3 steps, to ease a possible revert:
> > - kevent(2) refactoring
> > - introduction of newer kq* APIs
> > - dopselect rewrite
>
> Please send a separate patch for the first step.
Here's it. Diff below changes kevent(2) to possibly call kqueue_scan()
multiple times. The same pattern is/will be used by select(2) and
poll(2).
The copyout(2) and ktrace entry point have been moved to the syscall
function.
Comments? Oks?
Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.145
diff -u -p -r1.145 kern_event.c
--- kern/kern_event.c 25 Nov 2020 13:49:00 - 1.145
+++ kern/kern_event.c 30 Nov 2020 20:12:08 -
@@ -567,6 +567,7 @@ sys_kevent(struct proc *p, void *v, regi
struct timespec ts;
struct timespec *tsp = NULL;
int i, n, nerrors, error;
+ int ready, total;
struct kevent kev[KQ_NEVENTS];
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
@@ -595,9 +596,9 @@ sys_kevent(struct proc *p, void *v, regi
kq = fp->f_data;
nerrors = 0;
- while (SCARG(uap, nchanges) > 0) {
- n = SCARG(uap, nchanges) > KQ_NEVENTS ?
- KQ_NEVENTS : SCARG(uap, nchanges);
+ while ((n = SCARG(uap, nchanges)) > 0) {
+ if (n > nitems(kev))
+ n = nitems(kev);
error = copyin(SCARG(uap, changelist), kev,
n * sizeof(struct kevent));
if (error)
@@ -635,11 +636,36 @@ sys_kevent(struct proc *p, void *v, regi
kqueue_scan_setup(&scan, kq);
FRELE(fp, p);
- error = kqueue_scan(&scan, SCARG(uap, nevents), SCARG(uap, eventlist),
- tsp, kev, p, &n);
+ /*
+* Collect as many events as we can. The timeout on successive
+* loops is disabled (kqueue_scan() becomes non-blocking).
+*/
+ total = 0;
+ error = 0;
+ while ((n = SCARG(uap, nevents) - total) > 0) {
+ if (n > nitems(kev))
+ n = nitems(kev);
+ ready = kqueue_scan(&scan, n, kev, tsp, p, &error);
+ if (ready == 0)
+ break;
+ error = copyout(kev, SCARG(uap, eventlist) + total,
+ sizeof(struct kevent) * ready);
+#ifdef KTRACE
+ if (KTRPOINT(p, KTR_STRUCT))
+ ktrevent(p, kev, ready);
+#endif
+ total += ready;
+ if (error || ready < n)
+ break;
+ /*
+* Successive loops are only necessary if there are more
+* ready events to gather, so they don't need to block.
+*/
+ tsp = &ts;
+ timespecclear(tsp);
+ }
kqueue_scan_finish(&scan);
-
- *retval = n;
+ *retval = total;
return (error);
done:
@@ -893,22 +919,22 @@ kqueue_sleep(struct kqueue *kq, struct t
return (error);
}
+/*
+ * Scan the kqueue, blocking if necessary until the target time is reached.
+ * If tsp is NULL we block indefinitely. If tsp->ts_secs/nsecs are both
+ * 0 we do not block at all.
+ */
int
kqueue_scan(struct kqueue_scan_state *scan, int maxevents,
-struct kevent *ulistp, struct timespec *tsp, struct kevent *kev,
-struct proc *p, int *retval)
+struct kevent *kevp, struct timespec *tsp, struct proc *p, int *errorp)
{
struct kqueue *kq = scan->kqs_kq;
- struct kevent *kevp;
struct knote *kn;
- int s, count, nkev, error = 0;
+ int s, count, nkev = 0, error = 0;
- nkev = 0;
- kevp = kev;
count = maxevents;
if (count == 0)
goto done;
-
retry:
KASSERT(count == maxevents);
KASSERT(nkev == 0);
@@ -958,14 +984,8 @@ retry:
while (count) {
kn = TAILQ_NEXT(&scan->kqs_start, kn_tqe);
if (kn->kn_filter == EVFILT_MARKER) {
- if (kn == &scan->kqs_end) {
- TAILQ_REMOVE(&kq->kq_head, &scan->kqs_start,
- kn_tqe);
- splx(s);
- if (scan->kqs_nevent == 0)
- goto retry;
- goto done;
- }
+ if (kn == &scan->kqs_end)
+ break;
/* Move start marker past another thread's marker. */
TAILQ_REMOVE(&kq->kq_head, &scan->kqs_start, kn_tqe);
@@ -1001,6 +1021,9 @@ retry:
count--;
scan->kqs_nevent++;
+ /*
+* Post-event action on the note
+*/
Re: snmp trap usage
ok Martijn van Duren wrote: > I missed an argc check which causes snmp trap to segfault if called with > too few arguments instead of showing usage. > > OK? > > martijn@ > > Index: snmpc.c > === > RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v > retrieving revision 1.30 > diff -u -p -r1.30 snmpc.c > --- snmpc.c 14 Sep 2020 15:12:27 - 1.30 > +++ snmpc.c 30 Nov 2020 16:25:24 - > @@ -788,6 +788,9 @@ snmpc_trap(int argc, char *argv[]) > if (version == SNMP_V1) > errx(1, "trap is not supported for snmp v1"); > > + if (argc < 3) > + usage(); > + > if ((agent = snmpc_connect(argv[0], "162")) == NULL) > err(1, "%s", snmp_app->name); > > >
Re: rad(8): rdomain support
On Sun, Nov 29, 2020 at 12:20:31PM +0100, Florian Obser wrote:
>
> Let rad(8) handle all rdomains in a single daemon, similar to previous
> work in slaacd.
>
> Suggested / requested by tb who showed me previous work by reyk which
> unfortunately predated my work in slaacd and followed a different
> pattern.
>
> There has already been a fair bit of testing and input from tb on
> previous iterations.
>
> More tests, OKs?
>
Updated diff after some testing by tb@
- our AF_ROUTE socket needs a RTABLE_ANY ROUTE_TABLEFILTER otherwise
we don't see link-local addresses arriving in rdomains != 0.
- check if the arriving link-local address is tentative (or a
douplicate) and ignore it because we can't use it and sendmsg failes.
Monitor RTM_CHGADDRATTR messages to see when the link-local address
becomes valid
diff --git frontend.c frontend.c
index 8947f616e73..c9c63b0cb6b 100644
--- frontend.c
+++ frontend.c
@@ -95,19 +95,22 @@ struct icmp6_ev {
struct msghdrrcvmhdr;
struct iovec rcviov[1];
struct sockaddr_in6 from;
-} icmp6ev;
+ int refcnt;
+};
struct ra_iface {
- TAILQ_ENTRY(ra_iface) entry;
- struct ra_prefix_conf_head prefixes;
- charname[IF_NAMESIZE];
- charconf_name[IF_NAMESIZE];
- uint32_tif_index;
- int removed;
- int link_state;
- int prefix_count;
- size_t datalen;
- uint8_t data[RA_MAX_SIZE];
+ TAILQ_ENTRY(ra_iface)entry;
+ struct icmp6_ev *icmp6ev;
+ struct ra_prefix_conf_head prefixes;
+ char name[IF_NAMESIZE];
+ char conf_name[IF_NAMESIZE];
+ uint32_t if_index;
+ int rdomain;
+ int removed;
+ int link_state;
+ int prefix_count;
+ size_t datalen;
+ uint8_t data[RA_MAX_SIZE];
};
TAILQ_HEAD(, ra_iface) ra_interfaces;
@@ -119,6 +122,7 @@ void icmp6_receive(int, short, void
*);
voidjoin_all_routers_mcast_group(struct ra_iface *);
voidleave_all_routers_mcast_group(struct ra_iface *);
int get_link_state(char *);
+int get_ifrdomain(char *);
voidmerge_ra_interface(char *, char *);
voidmerge_ra_interfaces(void);
struct ra_iface*find_ra_iface_by_id(uint32_t);
@@ -127,6 +131,9 @@ struct ra_iface_conf*find_ra_iface_conf(struct
ra_iface_conf_head *,
char *);
struct ra_prefix_conf *find_ra_prefix_conf(struct ra_prefix_conf_head*,
struct in6_addr *, int);
+struct icmp6_ev*get_icmp6ev_by_rdomain(int);
+voidunref_icmp6ev(struct ra_iface *);
+voidset_icmp6sock(int, int);
voidadd_new_prefix_to_ra_iface(struct ra_iface *r,
struct in6_addr *, int, struct ra_prefix_conf *);
voidfree_ra_iface(struct ra_iface *);
@@ -147,7 +154,7 @@ struct rad_conf *frontend_conf;
struct imsgev *iev_main;
struct imsgev *iev_engine;
struct eventev_route;
-int icmp6sock = -1, ioctlsock = -1, routesock = -1;
+int ioctlsock = -1, routesock = -1;
struct ipv6_mreqall_routers;
struct sockaddr_in6 all_nodes;
struct msghdr sndmhdr;
@@ -175,8 +182,7 @@ frontend(int debug, int verbose)
{
struct event ev_sigint, ev_sigterm;
struct passwd *pw;
- size_t rcvcmsglen, sndcmsgbuflen;
- uint8_t *rcvcmsgbuf;
+ size_t sndcmsgbuflen;
uint8_t *sndcmsgbuf = NULL;
frontend_conf = config_new_empty();
@@ -229,20 +235,6 @@ frontend(int debug, int verbose)
iev_main->handler, iev_main);
event_add(&iev_main->ev, NULL);
- rcvcmsglen = CMSG_SPACE(sizeof(struct in6_pktinfo)) +
- CMSG_SPACE(sizeof(int));
- if((rcvcmsgbuf = malloc(rcvcmsglen)) == NULL)
- fatal("malloc");
-
- icmp6ev.rcviov[0].iov_base = (caddr_t)icmp6ev.answer;
- icmp6ev.rcviov[0].iov_len = sizeof(icmp6ev.answer);
- icmp6ev.rcvmhdr.msg_name = (caddr_t)&icmp6ev.from;
- icmp6ev.rcvmhdr.msg_namelen = sizeof(icmp6ev.from);
- icmp6ev.rcvmhdr.msg_iov = icmp6ev.rcviov;
- icmp6ev.rcvmhdr.msg_iovlen = 1;
Switch select(2) to kqueue-based implementation
Now that the kqueue refactoring has been committed, here's once again
the diff to modify the internal implementation of {p,}select(2) to query
kqfilter handlers instead of poll ones.
{p,}poll(2) are left untouched to ease the transition.
I plan to commit this in 3 steps, to ease a possible revert:
- kevent(2) refactoring
- introduction of newer kq* APIs
- dopselect rewrite
A mid-term goal of this change would be to get rid of the poll handlers
in order to have a single event system in the kernel to maintain and
turn mp-safe.
The logic is as follow:
- With this change every thread get a "private" kqueue, usable by the
kernel only, to register events for select(2) and later poll(2).
- Events specified via FD_SET(2) are converted to their kqueue equivalent.
- kqueue_scan() has been modified to be restartable and work with a given
kqueue.
- At the end of every {p,}select(2) syscall the private kqueue is purged.
This version should include your previous feedbacks.
Comments, tests and oks are welcome!
Thanks,
Martin
Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.145
diff -u -p -r1.145 kern_event.c
--- kern/kern_event.c 25 Nov 2020 13:49:00 - 1.145
+++ kern/kern_event.c 30 Nov 2020 15:30:40 -
@@ -57,6 +57,7 @@
#include
#include
+struct kqueue *kqueue_alloc(struct filedesc *);
void kqueue_terminate(struct proc *p, struct kqueue *);
void kqueue_free(struct kqueue *);
void kqueue_init(void);
@@ -504,6 +505,27 @@ const struct filterops dead_filtops = {
.f_event= filt_dead,
};
+void
+kqpoll_init(struct proc *p)
+{
+ if (p->p_kq != NULL)
+ return;
+
+ p->p_kq = kqueue_alloc(p->p_fd);
+ p->p_kq_serial = arc4random();
+}
+
+void
+kqpoll_exit(struct proc *p)
+{
+ if (p->p_kq == NULL)
+ return;
+
+ kqueue_terminate(p, p->p_kq);
+ kqueue_free(p->p_kq);
+ p->p_kq = NULL;
+}
+
struct kqueue *
kqueue_alloc(struct filedesc *fdp)
{
@@ -567,6 +589,7 @@ sys_kevent(struct proc *p, void *v, regi
struct timespec ts;
struct timespec *tsp = NULL;
int i, n, nerrors, error;
+ int ready, total;
struct kevent kev[KQ_NEVENTS];
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
@@ -595,9 +618,9 @@ sys_kevent(struct proc *p, void *v, regi
kq = fp->f_data;
nerrors = 0;
- while (SCARG(uap, nchanges) > 0) {
- n = SCARG(uap, nchanges) > KQ_NEVENTS ?
- KQ_NEVENTS : SCARG(uap, nchanges);
+ while ((n = SCARG(uap, nchanges)) > 0) {
+ if (n > nitems(kev))
+ n = nitems(kev);
error = copyin(SCARG(uap, changelist), kev,
n * sizeof(struct kevent));
if (error)
@@ -635,11 +658,36 @@ sys_kevent(struct proc *p, void *v, regi
kqueue_scan_setup(&scan, kq);
FRELE(fp, p);
- error = kqueue_scan(&scan, SCARG(uap, nevents), SCARG(uap, eventlist),
- tsp, kev, p, &n);
+ /*
+* Collect as many events as we can. The timeout on successive
+* loops is disabled (kqueue_scan() becomes non-blocking).
+*/
+ total = 0;
+ error = 0;
+ while ((n = SCARG(uap, nevents) - total) > 0) {
+ if (n > nitems(kev))
+ n = nitems(kev);
+ ready = kqueue_scan(&scan, n, kev, tsp, p, &error);
+ if (ready == 0)
+ break;
+ error = copyout(kev, SCARG(uap, eventlist) + total,
+ sizeof(struct kevent) * ready);
+#ifdef KTRACE
+ if (KTRPOINT(p, KTR_STRUCT))
+ ktrevent(p, kev, ready);
+#endif
+ total += ready;
+ if (error || ready < n)
+ break;
+ /*
+* Successive loops are only necessary if there are more
+* ready events to gather, so they don't need to block.
+*/
+ tsp = &ts;
+ timespecclear(tsp);
+ }
kqueue_scan_finish(&scan);
-
- *retval = n;
+ *retval = total;
return (error);
done:
@@ -893,22 +941,22 @@ kqueue_sleep(struct kqueue *kq, struct t
return (error);
}
+/*
+ * Scan the kqueue, blocking if necessary until the target time is reached.
+ * If tsp is NULL we block indefinitely. If tsp->ts_secs/nsecs are both
+ * 0 we do not block at all.
+ */
int
kqueue_scan(struct kqueue_scan_state *scan, int maxevents,
-struct kevent *ulistp, struct timespec *tsp, struct kevent *kev,
-struct proc *p, int *retval)
+struct kevent *kevp, struct timespec *tsp, struct proc *p, int *errorp)
{
struct kqueue *kq = scan->kqs_kq;
- struct kevent *kevp;
struct knote *kn;
- int s, count, nkev, error = 0;
+ int
snmp trap usage
I missed an argc check which causes snmp trap to segfault if called with too few arguments instead of showing usage. OK? martijn@ Index: snmpc.c === RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v retrieving revision 1.30 diff -u -p -r1.30 snmpc.c --- snmpc.c 14 Sep 2020 15:12:27 - 1.30 +++ snmpc.c 30 Nov 2020 16:25:24 - @@ -788,6 +788,9 @@ snmpc_trap(int argc, char *argv[]) if (version == SNMP_V1) errx(1, "trap is not supported for snmp v1"); + if (argc < 3) + usage(); + if ((agent = snmpc_connect(argv[0], "162")) == NULL) err(1, "%s", snmp_app->name);
Re: [PATCH] umb(4) fix for X20 (DW5821e) in Dell Latitude 7300
On Mon, Oct 19, 2020 at 08:11:04PM -0700, Bryan Vyhmeister wrote:
> On Fri, Oct 02, 2020 at 12:33:15PM -0700, Bryan Vyhmeister wrote:
> > On Wed, Sep 30, 2020 at 04:05:51PM -0700, Bryan Vyhmeister wrote:
> > > Gerhard Roth provided a patch to me to get the Qualcomm Snapdragon X20
> > > umb(4) interface in my Dell Latitude 7300 working. It is also known as a
> > > Dell DW5821e interface. I have been using this patch for months now with
> > > AT&T Wireless in the US and it works great just as I have been used to
> > > on my ThinkPad X270 with the older umb(4) interface. This is how it
> > > shows up now in my dmesg:
> > >
> > > umb0 at uhub0 port 16 "Dell Inc. DW5821e Snapdragon X20 LTE" rev
> > > 3.10/3.18 addr 2
> > >
> > > I would love to get this committed at some point so I no longer have to
> > > keep compiling a new kernel for every snapshot to have umb(4) working.
> >
> > Thanks to jmc@ for suggesting a man page diff as well. Both the diff for
> > if_umb.c and umb.4 are below.
>
> Ping.
Any further comments on this patch? I would love to see this fixed so
umb(4) on my laptop will work fine without a custom kernel. Thank you.
Bryan
Index: share/man/man4/umb.4
===
RCS file: /cvs/src/share/man/man4/umb.4,v
retrieving revision 1.11
diff -u -p -r1.11 umb.4
--- share/man/man4/umb.412 May 2020 13:03:52 - 1.11
+++ share/man/man4/umb.42 Oct 2020 19:30:53 -
@@ -44,6 +44,7 @@ PIN again even if the system was reboote
The following devices should work:
.Pp
.Bl -tag -width Ds -offset indent -compact
+.It Dell DW5821e
.It Ericsson H5321gw and N5321gw
.It Fibocom L831-EAU
.It Medion Mobile S4222 (MediaTek OEM)
Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.36
diff -u -p -r1.36 if_umb.c
--- sys/dev/usb/if_umb.c22 Jul 2020 02:16:02 - 1.36
+++ sys/dev/usb/if_umb.c2 Oct 2020 19:30:53 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_umb.c,v 1.36 2020/07/22 02:16:02 dlg Exp $ */
+/* $OpenBSD: if_umb.c,v 1.34 2020/05/04 14:41:03 gerhard Exp $ */
/*
* Copyright (c) 2016 genua mbH
@@ -225,6 +225,26 @@ const struct cfattach umb_ca = {
int umb_delay = 4000;
/*
+ * Normally, MBIM devices are detected by their interface class and subclass.
+ * But for some models that have multiple configurations, it is better to
+ * match by vendor and product id so that we can select the desired
+ * configuration ourselves.
+ *
+ * OTOH, some devices identifiy themself als an MBIM device but fail to speak
+ * the MBIM protocol.
+ */
+struct umb_products {
+ struct usb_devno dev;
+ int confno;
+};
+const struct umb_products umb_devs[] = {
+ { { USB_VENDOR_DELL, 0x81d7 }, 2 }, /* XXX FIXME */
+};
+
+#define umb_lookup(vid, pid) \
+ ((const struct umb_products *)usb_lookup(umb_devs, vid, pid))
+
+/*
* These devices require an "FCC Authentication" command.
*/
const struct usb_devno umb_fccauth_devs[] = {
@@ -263,6 +283,8 @@ umb_match(struct device *parent, void *m
struct usb_attach_arg *uaa = aux;
usb_interface_descriptor_t *id;
+ if (umb_lookup(uaa->vendor, uaa->product) != NULL)
+ return UMATCH_VENDOR_PRODUCT;
if (!uaa->iface)
return UMATCH_NONE;
if ((id = usbd_get_interface_descriptor(uaa->iface)) == NULL)
@@ -315,6 +337,43 @@ umb_attach(struct device *parent, struct
sc->sc_ctrl_ifaceno = uaa->ifaceno;
ml_init(&sc->sc_tx_ml);
+ if (uaa->configno < 0) {
+ /*
+* In case the device was matched by VID/PID instead of
+* InterfaceClass/InterfaceSubClass, we have to pick the
+* correct configuration ourself.
+*/
+ uaa->configno = umb_lookup(uaa->vendor, uaa->product)->confno;
+ DPRINTF("%s: switching to config #%d\n", DEVNAM(sc),
+ uaa->configno);
+ status = usbd_set_config_no(sc->sc_udev, uaa->configno, 1);
+ if (status) {
+ printf("%s: failed to switch to config #%d: %s\n",
+ DEVNAM(sc), uaa->configno, usbd_errstr(status));
+ goto fail;
+ }
+ usbd_delay_ms(sc->sc_udev, 200);
+
+ /*
+* Need to do some manual setups that usbd_probe_and_attach()
+* would do for us otherwise.
+*/
+ uaa->nifaces = uaa->device->cdesc->bNumInterface;
+ for (i = 0; i < uaa->nifaces; i++) {
+ if (usbd_iface_claimed(sc->sc_udev, i))
+ continue;
+ id =
usbd_get_interface_descriptor(&uaa->device->ifaces[i]);
+ if (id != NULL && id->bInterf
libressl pc files
Several ports are patched to cope with the version number in pkgconfig
files for libressl libs (currently all at 1.0.0)
portoriginally wanted
lang/php/7.2>= 1.0.1
lang/php/7.3>= 1.0.1
lang/php/7.4>= 1.0.1
multimedia/xine-lib >= 1.0.2
security/opensc >= 1.0.1
www/e2guardian >= 1.0.1
textproc/mupdf >= 1.1.0
instrumenting pkg-config these are the rest of what's wanted:
audio/pulseaudio> 0.9
emulators/retroarch >= 1.0.0
games/warzone2100 >= 1.0.0
mail/opendkim >= 0.9.7
multimedia/gstreamer-0.10/plugins-bad,-main >= 0.9.5
net/openvpn >= 1.0.2
net/qbittorrent/qbittorrent-nox >= 1.0
net/qbittorrent/qbittorrent >= 1.0
net/seafile/client >= 0.98
net/transmission,-gtk >= 0.9.7
print/qpdf >= 1.1.0
productivity/grisbi >= 1.0.0
security/libbde >= 1.0
security/libewf >= 1.0
security/xca>= 0.9.8
security/xmlsec,-main >= 1.0.0
sysutils/libfsapfs >= 1.0
sysutils/libfvde>= 1.0
sysutils/libluksde >= 1.0
sysutils/libqcow>= 1.0
sysutils/libsmraw >= 1.0
sysutils/syslog-ng >= 0.9.8
www/aria2 >= 0.9.8
www/nghttp2 >= 1.0.1
x11/remmina >= 1.0.0
x11/virt-viewer >= 1.0.0
x11/x11vnc >= 1.0.0
I'm wondering if it would make sense to bump the advertised
version so that at least the >= 1.0.2 checks work. Though the only
things in ports wanting 1.1.0 (mupdf, qpdf) are happy building against
libressl anyway so perhaps we should just change it to 2.0.0 to match
LIBRESSL_VERSION_NUMBER. (Typically pkg-config checks are just used
to detect presence of a suitable version, not to choose codepaths
for ifdefs).
Currently the version in pc files comes from SHLIB_VERSION_NUMBER
in libcrypto/opensslv.h; however that is in a section which says
"These will never change" so if we want to keep that promise then
perhaps the generate_pkgconfig.sh scripts should generate the
number for the pc files directly.
Any OKs for either of these?
Index: libcrypto/opensslv.h
===
RCS file: /cvs/src/lib/libcrypto/opensslv.h,v
retrieving revision 1.62
diff -u -p -u -1 -2 -r1.62 opensslv.h
--- libcrypto/opensslv.h18 Nov 2020 11:10:08 - 1.62
+++ libcrypto/opensslv.h25 Nov 2020 20:28:56 -
@@ -4,15 +4,15 @@
/* These will change with each release of LibreSSL-portable */
#define LIBRESSL_VERSION_NUMBER 0x303fL
/*^ Patch starts here */
#define LIBRESSL_VERSION_TEXT "LibreSSL 3.3.0"
/* These will never change */
#define OPENSSL_VERSION_NUMBER 0x2000L
#define OPENSSL_VERSION_TEXT LIBRESSL_VERSION_TEXT
#define OPENSSL_VERSION_PTEXT " part of " OPENSSL_VERSION_TEXT
#define SHLIB_VERSION_HISTORY ""
-#define SHLIB_VERSION_NUMBER "1.0.0"
+#define SHLIB_VERSION_NUMBER "2.0.0"
#endif /* HEADER_OPENSSLV_H */
or just changing generate_pkgconfig (either like this, or alternatively
ripping more out):
Index: libcrypto/generate_pkgconfig.sh
===
RCS file: /cvs/src/lib/libcrypto/generate_pkgconfig.sh,v
retrieving revision 1.2
diff -u -p -r1.2 generate_pkgconfig.sh
--- libcrypto/generate_pkgconfig.sh 3 Sep 2016 12:42:46 - 1.2
+++ libcrypto/generate_pkgconfig.sh 25 Nov 2020 20:30:06 -
@@ -53,7 +53,8 @@ fi
version_re="s/^#define[[:blank:]]+SHLIB_VERSION_NUMBER[[:blank:]]+\"(.*)\".*/\1/p"
#version_file=${curdir}/src/crypto/opensslv.h
version_file=${curdir}/opensslv.h
-lib_version=$(sed -nE ${version_re} ${version_file})
+#lib_version=$(sed -nE ${version_re} ${version_file})
+lib_version="2.0.0"
# Put -I${includedir} into Cflags so configure script tests like
# test -n "`pkg-config --cflags openssl`"
Index: libssl/generate_pkgconfig.sh
===
RCS file: /cvs/src/lib/libssl/generate_pkgconfig.sh,v
retrieving revision 1.9
diff -u -p -r1.9 generate_pkgconfig.sh
--- libssl/generate_pkgconfig.sh3 Sep 2016 12:42:42 - 1.9
+++ libssl/generate_pkgconfig.sh25 Nov 2020 20:30:06 -
@@ -52,7 +52,8 @@ fi
version_re="s/^#define[[:blank:]]+SHLIB_VERSION_NUMBER[[:blank:]]+\"(.*)\".*/\1/p"
version_file=${curdir}/../libcrypto/opensslv.h
-lib_version=$(sed -nE ${version_re} ${version_file})
+#lib_version=$(sed -nE ${version_re} ${version_file})
+lib_version="2.0.0"
# Put -I${includedir} into Cflags so configure script tests like
# test -n "`pkg-config --cflags openssl`"
Re: tcpdump: use unsigned char in isprint
On Sun, Nov 29, 2020 at 07:21:53PM -0800, Greg Steuck wrote:
> Vitaliy Makkoveev writes:
>
> > Hi.
> >
> > For me `ch' is unwanted here.
> >
> > Index: usr.sbin/tcpdump/util.c
> > ===
> > RCS file: /cvs/src/usr.sbin/tcpdump/util.c,v
> > retrieving revision 1.30
> > diff -u -p -r1.30 util.c
> > --- usr.sbin/tcpdump/util.c 24 Jan 2020 22:46:37 - 1.30
> > +++ usr.sbin/tcpdump/util.c 29 Nov 2020 22:56:23 -
> > @@ -303,13 +303,11 @@ safeputs(const char *s)
> > void
> > safeputchar(int c)
> > {
> > - unsigned char ch;
> > -
> > - ch = (unsigned char)(c & 0xff);
> > + c &= 0xff;
> > if (c < 0x80 && isprint(c))
> > - printf("%c", c & 0xff);
> > + printf("%c", c);
>
> In the name of bike-shedding this should be a putchar, then OK gnezdo@
It makes sence.
Index: usr.sbin/tcpdump/util.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/util.c,v
retrieving revision 1.30
diff -u -p -r1.30 util.c
--- usr.sbin/tcpdump/util.c 24 Jan 2020 22:46:37 - 1.30
+++ usr.sbin/tcpdump/util.c 30 Nov 2020 10:02:25 -
@@ -303,13 +303,11 @@ safeputs(const char *s)
void
safeputchar(int c)
{
- unsigned char ch;
-
- ch = (unsigned char)(c & 0xff);
+ c &= 0xff;
if (c < 0x80 && isprint(c))
- printf("%c", c & 0xff);
+ putchar(c);
else
- printf("\\%03o", c & 0xff);
+ printf("\\%03o", c);
}
/*
Re: relayd and TLS client cert verification
Hi! > I have patch on top of this which allows to pass remote certificate > and/or parts of it to backend hosts via http headers. Did this patch ever arrive and would it also make sense inside httpd (in addition to relayd)? -- Markus Läll
Re: amdgpu(4): use DRM_INFO instead of printf for printing compute unit info
On Mon, Nov 30, 2020 at 12:02:15AM -0600, Charlie Burnett wrote:
> Doesn’t DRM_INFO output even if DRMDEBUG is enabled? I thought DRM_DEBUG
> only output when debug’s enabled while DRM_INFO is pretty much just a
> wrapper for printk?
Currently DRM_INFO will call into printk which returns early for
KERN_INFO messages if DRMDEBUG is not set, see printk in drm_linux.c.
>
> Either way, I found the call writing to the GPU register it doesn’t like
> earlier this weekend, but haven’t figured out what exactly it’s having
> issues with. Have a busy week or two but I’ll try to come back to it then!
> Just wanted to send something in case anyone else had a similar issue.
Including the error message you saw would be helpful.
>
> On Sun, Nov 29, 2020 at 11:25 PM Jonathan Gray wrote:
>
> > On Sun, Nov 29, 2020 at 10:17:22PM -0600, Charlie Burnett wrote:
> > > Howdy all,
> > > For reasons that are beyond me, when printf is called in amdgpu_device.c
> > to
> > > print the CU info, it gives me a psp firmware load failure on a Radeon
> > VII
> > > (Vega 20) gpu. Switching the printf statement to a DRM_INFO statement as
> > > used in the rest of amdgpu seems to fix it though.
> >
> > Find what is sensitive to the delay.
> >
> > Hiding this printf under DRMDEBUG makes no sense. You should be able to
> > load kernels with and without DRMDEBUG.
> >
> > >
> > > ok?
> > >
> > > diff --git sys/dev/pci/drm/amd/amdgpu/amdgpu_device.c
> > > sys/dev/pci/drm/amd/amdgpu/amdgpu_device.c
> > > index 45eff483e86..fba5a7caf23 100644
> > > --- sys/dev/pci/drm/amd/amdgpu/amdgpu_device.c
> > > +++ sys/dev/pci/drm/amd/amdgpu/amdgpu_device.c
> > > @@ -3210,7 +3210,7 @@ fence_driver_init:
> > > default:
> > > chip_name = amdgpu_asic_name[adev->asic_type];
> > > }
> > > - printf("%s: %s %d CU rev 0x%02x\n", adev->self.dv_xname,
> > > + DRM_INFO( "%s: %s %d CU rev 0x%02x\n", adev->self.dv_xname,
> > > chip_name, adev->gfx.cu_info.number, adev->rev_id);
> > > }
> > > #endif
> > >
> >
> >
>
