Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
> On Jul 5, 2022, at 23:02, Mohamed Aslan wrote: > > Hi, > > Apologies. My bad, I applied the latest patch but booted into another > kernel with an earlier patch! > > Here's what I got with your latest patch: > > $ dmesg | grep 'tsc' > tsc: cpu0/cpu1: sync test round 1/2 failed > tsc:

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
> On Jul 5, 2022, at 21:31, Mohamed Aslan wrote: > > Hello, > > I just tested your patch on my lenovo e495 laptop, unfortunately > still no tsc. > > $ dmesg | grep 'tsc:' > tsc: cpu0/cpu1 sync round 1: 20468 regressions > tsc: cpu0/cpu1 sync round 1: cpu0 lags cpu1 by 5351060292 cycles > tsc:

Re: powerpc, macppc: retrigger deferred DEC interrupts from splx(9)

2022-07-05 Thread Scott Cheloha
On Thu, Jun 23, 2022 at 09:58:48PM -0500, Scott Cheloha wrote: > > [...] > > Thoughts? Tweaks? > > [...] miod: Any issues? kettenis: Anything to add? ok? drahn: Anything to add? ok? -- It would be nice (but not strictly necessary) to test this on a machine doing &quo

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
On Wed, Jul 06, 2022 at 09:14:03AM +0900, Yuichiro NAITO wrote: > Hi, Scott. > > I tested your patch on my OpenBSD running on ESXi. > It works fine for me and I never see monotonic clock going backward. > There is nothing extra messages in my dmesg. Great! Thanks for testing.

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
On Tue, Jul 05, 2022 at 01:38:32PM -0700, Mike Larkin wrote: > On Mon, Jul 04, 2022 at 09:06:55PM -0500, Scott Cheloha wrote: > > > > [...] > > Here's the output from a 4 socket 80 thread machine. Oh nice. I think this is the biggest machine we've tried so far. > kern

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
On Tue, Jul 05, 2022 at 06:40:26PM +0200, Stuart Henderson wrote: > On 2022/07/05 11:22, Scott Cheloha wrote: > > On Tue, Jul 05, 2022 at 05:47:51PM +0200, Stuart Henderson wrote: > > > On 2022/07/04 21:06, Scott Cheloha wrote: > > > > 4. OpenBSD VMs on other

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
On Tue, Jul 05, 2022 at 05:47:51PM +0200, Stuart Henderson wrote: > On 2022/07/04 21:06, Scott Cheloha wrote: > > 4. OpenBSD VMs on other hypervisors. > > KVM on proxmox VE 7.1-12 > > I force acpihpet0 on this; it defaults to pvclock which results in > timekeeping so bad

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
On Tue, Jul 05, 2022 at 05:38:04PM +0200, Stuart Henderson wrote: > On 2022/07/04 21:06, Scott Cheloha wrote: > > 2. Other multisocket machines. > > This is from the R620 where I originally discovered the problems > with SMP with the previous TSC test: > > $ d

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
On Tue, Jul 05, 2022 at 10:53:43AM -0400, Dave Voutila wrote: > > Scott Cheloha writes: > > > On Tue, Jul 05, 2022 at 07:15:31AM -0400, Dave Voutila wrote: > >> > >> Scott Cheloha writes: > >> > >> > [...] > >> > > >>

Re: ts(1): make timespec-handling code more obvious

2022-07-05 Thread Scott Cheloha
On Tue, Jul 05, 2022 at 11:53:26AM +0200, Claudio Jeker wrote: > On Tue, Jul 05, 2022 at 11:34:21AM +, Job Snijders wrote: > > On Tue, Jul 05, 2022 at 11:08:13AM +0200, Claudio Jeker wrote: > > > On Mon, Jul 04, 2022 at 05:10:05PM -0500, Scott Cheloha wrote: > > > &

Re: [v3] amd64: simplify TSC sync testing

2022-07-05 Thread Scott Cheloha
On Tue, Jul 05, 2022 at 07:15:31AM -0400, Dave Voutila wrote: > > Scott Cheloha writes: > > > [...] > > > > If you fail the test you will see something like this: > > > > tsc: cpu0/cpu2: sync test round 1/2 failed > > tsc: cpu0/cpu2: cpu2:

[v3] amd64: simplify TSC sync testing

2022-07-04 Thread Scott Cheloha
Hi, Once again, I am trying to change our approach to TSC sync testing to eliminate false positive results. Instead of trying to repair the TSC by measuring skew, we just spin in a lockless loop looking for skew and mark the TSC as broken if we detect any. This is motivated in part by some

Re: ts(1): make timespec-handling code more obvious

2022-07-04 Thread Scott Cheloha
On Mon, Jul 04, 2022 at 11:15:24PM +0200, Claudio Jeker wrote: > On Mon, Jul 04, 2022 at 01:28:12PM -0500, Scott Cheloha wrote: > > Hi, > > > > Couple things: > > > > [...] > > I don't like the introduction of all these local variables that are just >

ts(1): make timespec-handling code more obvious

2022-07-04 Thread Scott Cheloha
Hi, Couple things: - Use additional timespec variables to make our intent more obvious. Add "elapsed", "utc_offset", and "utc_start". "roff" is a confusing name, "utc_offset" is better. Yes, I know the clock is called CLOCK_REALTIME, but that's a historical accident. Ideally they

Re: powerpc, macppc: retrigger deferred DEC interrupts from splx(9)

2022-06-29 Thread Scott Cheloha
On Wed, Jun 29, 2022 at 10:34:53PM -0400, George Koehler wrote: > Hi. I have a question about splx, below. > > On Thu, 23 Jun 2022 21:58:48 -0500 > Scott Cheloha wrote: > > > My machine uses openpic(4). I would appreciate tests on a > > non-openpic(4) Mac, thoug

Re: start unlocking kbind(2)

2022-06-24 Thread Scott Cheloha
On Wed, Jun 15, 2022 at 10:40:35PM -0500, Scott Cheloha wrote: > On Wed, Jun 15, 2022 at 06:17:07PM -0600, Theo de Raadt wrote: > > Mark Kettenis wrote: > > > > > Well, I believe that Scott was trying to fix a race condition that can > > > only happen if code i

powerpc, macppc: retrigger deferred DEC interrupts from splx(9)

2022-06-23 Thread Scott Cheloha
Hi, One of the problems obstructing my dynamic clock interrupt patch is that clock interrupts on powerpc don't (can't?) behave the same as clock interrupts on amd64, arm64, and sparc64. In particular, for historical reasons, on powerpc you cannot mask decrementer (DEC) interrupts without *also*

kernel: remove global "randompid" toggle

2022-06-16 Thread Scott Cheloha
All PIDs after we fork init(8) are random. This has been the case for over 8 years: https://cvsweb.openbsd.org/src/sys/kern/init_main.c?rev=1.193=text/x-cvsweb-markup Are we keeping this "randompid" global around to make it possible to disable random PIDs by toggling it in ddb(4)? Maybe we

Re: start unlocking kbind(2)

2022-06-15 Thread Scott Cheloha
On Wed, Jun 15, 2022 at 06:17:07PM -0600, Theo de Raadt wrote: > Mark Kettenis wrote: > > > Well, I believe that Scott was trying to fix a race condition that can > > only happen if code is using kbind(2) incorrectly, i.e. when the > > threads deliberately pass different cookies to kbind(2) or

Re: start unlocking kbind(2)

2022-06-13 Thread Scott Cheloha
On Sun, Jun 12, 2022 at 12:12:33AM -0600, Theo de Raadt wrote: > David Gwynne wrote: > > > On Wed, May 18, 2022 at 07:42:32PM -0600, Theo de Raadt wrote: > > > Mark Kettenis wrote: > > > > > > > > Isn't the vm_map_lock enough? > > > > > > > > Could be. The fast path is going to take that

Re: powerpc64: do tc_init(9) before cpu_startclock()

2022-05-26 Thread Scott Cheloha
> On May 24, 2022, at 7:12 PM, Scott Cheloha wrote: > > In the future, the clock interrupt will need a working timecounter to > accurately reschedule itself. > > Move tc_init(9) up before cpu_startclock(). > > (I can't test this but it seems correct.) > > ok

powerpc64: do tc_init(9) before cpu_startclock()

2022-05-24 Thread Scott Cheloha
In the future, the clock interrupt will need a working timecounter to accurately reschedule itself. Move tc_init(9) up before cpu_startclock(). (I can't test this but it seems correct.) ok? Index: clock.c === RCS file:

librthread: validate timespec inputs with timespecisvalid(3)

2022-05-14 Thread Scott Cheloha
ok? Index: rthread_rwlock_compat.c === RCS file: /cvs/src/lib/librthread/rthread_rwlock_compat.c,v retrieving revision 1.1 diff -u -p -r1.1 rthread_rwlock_compat.c --- rthread_rwlock_compat.c 13 Feb 2019 13:15:39 - 1.1

Re: amd64: do CPU identification before TSC sync test

2022-05-11 Thread Scott Cheloha
> On May 11, 2022, at 2:51 AM, Yuichiro NAITO wrote: > > On 5/11/22 14:34, Yuichiro NAITO wrote: >> After applying your patch, cpu1 is not identified on my current kernel. >> Dmesg shows as follows. I'll see it further more. > > I found that LAPIC is necessary for `delay` function that is used

Re: [v2] amd64: simplify TSC sync testing

2022-05-10 Thread Scott Cheloha
On Wed, May 11, 2022 at 10:52:55AM +0900, Yuichiro NAITO wrote: > Hi, Scott. > > Recently I started running OpenBSD on ESXi. > I'm facing monotonic time going back problem as same as Yasuoka-san's report. > > https://marc.info/?l=openbsd-tech=161657532610882=2 > > I've tried your v2 patch. It

Re: amd64: do CPU identification before TSC sync test

2022-05-10 Thread Scott Cheloha
On Tue, Mar 29, 2022 at 10:24:03AM -0500, Scott Cheloha wrote: > On Tue, Mar 29, 2022 at 03:26:49PM +1100, Jonathan Gray wrote: > > On Mon, Mar 28, 2022 at 10:52:09PM -0500, Scott Cheloha wrote: > > > I want to use the IA32_TSC_ADJUST MSR where available when testing TSC > >

Re: ratecheck mutex

2022-05-04 Thread Scott Cheloha
> On May 3, 2022, at 17:16, Alexander Bluhm wrote: > > Hi, > > We have one comment that locking for ratecheck(9) is missing. In > all other places locking status of the struct timeval *lasttime > is unclear. > > The easiest fix is a global mutex for all lasttime in ratecheck(). > This covers

Re: kstat(1): implement wait with setitimer(2)

2022-05-02 Thread Scott Cheloha
On Sat, Apr 30, 2022 at 01:27:44AM +0200, Alexander Bluhm wrote: > On Thu, Apr 28, 2022 at 08:54:02PM -0500, Scott Cheloha wrote: > > On Thu, Sep 17, 2020 at 06:29:48PM -0500, Scott Cheloha wrote: > > > [...] > > > > > > Using nanosleep(2) to print the

Re: speaker(4): unhook driver and manpage from build

2022-04-29 Thread Scott Cheloha
> On Apr 29, 2022, at 10:40, Jeremie Courreges-Anglas wrote: > > On Thu, Apr 28 2022, Scott Cheloha wrote: >> speaker(4) is a whimsical thing, but I don't think we should have a >> dedicated chiptune interpreter in the kernel. > >> This patch unhooks the driver

Re: speaker(4): unhook driver and manpage from build

2022-04-29 Thread Scott Cheloha
> On Apr 29, 2022, at 09:33, Angelo wrote: > > Hello > >> On Thu, Apr 28, 2022 at 06:34:00AM -0500, Scott Cheloha wrote: >> speaker(4) is a whimsical thing, but I don't think we should have a >> dedicated chiptune interpreter in the kernel. >> >> Thi

Re: speaker(4): unhook driver and manpage from build

2022-04-29 Thread Scott Cheloha
> On Apr 29, 2022, at 10:06, j...@entropicblur.com wrote: > > On 2022-04-29 08:31, Angelo wrote: >> Hello >>> On Thu, Apr 28, 2022 at 06:34:00AM -0500, Scott Cheloha wrote: >>> speaker(4) is a whimsical thing, but I don't think we should have a >>> de

Re: timecounting: use full 96-bit product when computing high-res time

2022-04-29 Thread Scott Cheloha
On Thu, Oct 14, 2021 at 04:13:18PM -0500, Scott Cheloha wrote: > > [...] > > When we compute high resolution time, both in the kernel and in libc, > we get a 32-bit (or smaller) value from the active timecounter and > scale it up into a 128-bit bintime. > > The scaling m

Re: kstat(1): implement wait with setitimer(2)

2022-04-28 Thread Scott Cheloha
On Thu, Sep 17, 2020 at 06:29:48PM -0500, Scott Cheloha wrote: > [...] > > Using nanosleep(2) to print the stats periodically causes the period > to drift. If you use setitimer(2) it won't drift. > > ok? 19 month bump and rebase. I have updated the patch according to inp

speaker(4): unhook driver and manpage from build

2022-04-28 Thread Scott Cheloha
speaker(4) is a whimsical thing, but I don't think we should have a dedicated chiptune interpreter in the kernel. This patch unhooks the driver and the manpage from the build. The driver is built for alpha, amd64, and i386. A subsequent patch will move all relevant files to the attic and clean

Re: amd64: do CPU identification before TSC sync test

2022-03-29 Thread Scott Cheloha
On Tue, Mar 29, 2022 at 03:26:49PM +1100, Jonathan Gray wrote: > On Mon, Mar 28, 2022 at 10:52:09PM -0500, Scott Cheloha wrote: > > I want to use the IA32_TSC_ADJUST MSR where available when testing TSC > > synchronization. We note if it's available during CPU identification. >

amd64: do CPU identification before TSC sync test

2022-03-28 Thread Scott Cheloha
I want to use the IA32_TSC_ADJUST MSR where available when testing TSC synchronization. We note if it's available during CPU identification. Can we do CPU identification earlier in cpu_hatch() and cpu_start_secondary(), before we do the TSC sync testing? This can wait until after release. I'm

ssh: xstrdup(): use memcpy(3)

2022-03-09 Thread Scott Cheloha
The strdup(3) implementation in libc uses memcpy(3), not strlcpy(3). There is no upside to using strlcpy(3) here if we know the length of str before we copy it to the destination buffer. ... unless we're worried the length of str will change? Which would be very paranoid. But if that's the

[v2] amd64: simplify TSC sync testing

2022-02-22 Thread Scott Cheloha
Hi, Here is a second draft patch for changing our approach to TSC synchronization. With this patch, instead of trying to fix desync with a handshake we test for desync with a (more) foolproof loop and then don't attempt to correct for desync if we detect it. The motivation for a more foolproof

look(1): drop "rpath" promise after open(2)/fstat(2)

2022-02-08 Thread Scott Cheloha
The look(1) program needs to open(2) and fstat(2) exactly one file during its runtime. Using unveil(2) seems like overkill here. This seems closer to what we want: - pledge(2) initially with "stdio rpath" at the top of main(). We know we need to read a file at this point but don't yet know

Re: tr(1): improve table names

2022-02-08 Thread Scott Cheloha
On Sun, Jan 30, 2022 at 10:23:43AM -0600, Scott Cheloha wrote: > In tr(1), we have these two global arrays, "string1" and "string2". > > I have a few complaints: > > 1. They are not strings. They are lookup tables. The names are >misleading. >

rev(1): drop "rpath" promise in no-file branch

2022-02-08 Thread Scott Cheloha
We don't need "rpath" if we're only processing the standard input. ok? Index: rev.c === RCS file: /cvs/src/usr.bin/rev/rev.c,v retrieving revision 1.15 diff -u -p -r1.15 rev.c --- rev.c 29 Jan 2022 00:11:54 - 1.15 +++

Re: head(1): check for stdio errors

2022-02-06 Thread Scott Cheloha
> On Feb 6, 2022, at 20:07, Todd C. Miller wrote: > > Since the input is opened read-only I don't see the point in checking > the fclose() return value. However, if you are going to do so, you > might as well combine it with the ferror() check. E.g. > >if (ferror(fp) || fclose(fp) ==

head(1): check for stdio errors

2022-02-06 Thread Scott Cheloha
Add missing stdio error checks to head(1): - Output errors are terminal. The output is always stdout. - Input errors yield a warning and cause the program to fail gracefully. - Restructure the getc(3)/putchar(3) loop in head_file() to accomodate checking for errors. ok? P.S.

Re: amd64: simplify TSC sync testing

2022-02-02 Thread Scott Cheloha
> On Feb 2, 2022, at 13:29, Stuart Henderson wrote: > > Thanks for testing. > >> On 2022/02/02 13:51, Dave Voutila wrote: >> >> Jason McIntyre writes: >> >>> On Wed, Feb 02, 2022 at 04:52:40PM +, Stuart Henderson wrote: This definitely wants testing on Ryzen ThinkPads (e.g.

Re: cat(1): drop "rpath" promise in no-file case

2022-02-01 Thread Scott Cheloha
On Tue, Feb 01, 2022 at 01:33:19PM -0700, Todd C. Miller wrote: > On Tue, 01 Feb 2022 09:59:51 -0600, Scott Cheloha wrote: > > > To recap: > > > > - Refactor the open/close portions of cook_args() and raw_args() into a > >single function, cat_file(). > &g

Re: cat(1): drop "rpath" promise in no-file case

2022-02-01 Thread Scott Cheloha
On Wed, Dec 15, 2021 at 11:51:48AM +, Ricardo Mestre wrote: > > [...] > > the filename parameter on cook_buf is just used to show a warning, but it > should > be "stdin" instead of "(stdin)" to keep the same old behaviour. Yep, fixed. > additionally, please add a blank line after the

tr(1): improve table names

2022-01-30 Thread Scott Cheloha
In tr(1), we have these two global arrays, "string1" and "string2". I have a few complaints: 1. They are not strings. They are lookup tables. The names are misleading. 2. The arguments given to tr(1) in argv[] are indeed called "string1" and "string2". These are the names used in the

Re: touch(1): don't leak descriptor if futimens(2) fails

2022-01-28 Thread Scott Cheloha
On Fri, Jan 28, 2022 at 07:28:40AM -0700, Todd C. Miller wrote: > On Thu, 27 Jan 2022 20:02:18 -0800, Philip Guenther wrote: > > > > I think futimens(2) and close(2) failures are exotic enough to warrant > > > printing the system call name. > > > > I don't understand this. Can you give an

touch(1): don't leak descriptor if futimens(2) fails

2022-01-27 Thread Scott Cheloha
If futimens(2) fails here then close(2) is not called and we leak the descriptor. I think futimens(2) and close(2) failures are exotic enough to warrant printing the system call name. ok? Index: touch.c === RCS file:

amd64: simplify TSC sync testing

2022-01-27 Thread Scott Cheloha
Hi, sthen@ complained recently about a multisocket system not being able to use the TSC in userspace because the sync test measured too much skew and disabled it. I don't think there is any real skew on that system. I think the sync test is confusing NUMA overhead for skew and issuing a false

rev(1): refactor main loop

2022-01-27 Thread Scott Cheloha
Just like with head(1), rev(1)'s main loop is obfuscated. This patch moves the open/read/write/close portion of the main loop out of main() into a separate function, rev_file(). "multibyte" becomes a global. I think the result is easier to understand at a glance. In a subsequent patch I want

head(1): refactor main loop

2022-01-26 Thread Scott Cheloha
The main loop in head(1) is obfuscated. In particular, the path through the loop to exit(3) is extremely clever. Clever in a bad way. This patch moves the open/read/write/close portions of the loop out into a separate function, head_file(). I think the result is easier to understand. We only

Re: rev(1): pull MB_CUR_MAX out of the hot loop

2022-01-07 Thread Scott Cheloha
On Fri, Jan 07, 2022 at 01:43:24PM -0600, Scott Cheloha wrote: > > [...] > > Like this? > > [...] Updated: make the for-loop update expressions match. Index: rev.c === RCS file: /cvs/src/usr.bin/rev/rev.c,v r

Re: rev(1): pull MB_CUR_MAX out of the hot loop

2022-01-07 Thread Scott Cheloha
On Fri, Jan 07, 2022 at 07:28:30PM +0100, Ingo Schwarze wrote: > > [...] > > Scott Cheloha wrote on Fri, Jan 07, 2022 at 09:45:41AM -0600: > > > In rev(1), we call MB_CUR_MAX for every byte in the input stream. > > This is extremely expensive. > > [...] >

rev(1): pull MB_CUR_MAX out of the hot loop

2022-01-07 Thread Scott Cheloha
In rev(1), we call MB_CUR_MAX for every byte in the input stream. This is extremely expensive. It is much cheaper to call it once per line and use a simpler loop (see the inlined patch below) if the current locale doesn't handle multibyte characters: # -current rev(1), $ for i in $(jot 10); do

uniq(1): freopen(3) stdin, stdout

2021-12-29 Thread Scott Cheloha
uniq(1) defaults to using stdin and stdout, has exactly one input and one output, and allows changing the input and/or output to a given file. So, freopen(3) is the idiomatic thing to use here. It simplifies and shortens the code. Then we don't need the file() function, we don't need additional

uniq(1): don't skip() lines more than once

2021-12-14 Thread Scott Cheloha
In uniq(1), calling skip() to skip fields and/or characters on each input line is extremely expensive. One way to reduce the cost is to only do it once for a given line, instead of doing it repeatedly for the most recent unique line. The performance improvement for this trivial change is

cat(1): drop "rpath" promise in no-file case

2021-12-13 Thread Scott Cheloha
We don't need the "rpath" promise if there are no file arguments. We drop this promise in other utilities that default to stdin in the no-file case. If we consolidate raw_args() and cook_args() into a single function, cat_file(), we can drop the "rpath" promise in main(), in a single place in

Re: cat(1): always use a 64K buffer

2021-12-13 Thread Scott Cheloha
> On Dec 13, 2021, at 01:13, Otto Moerbeek wrote: > > On Sun, Dec 12, 2021 at 07:15:51PM -0600, Scott Cheloha wrote: > >> cat(1) sizes its I/O buffer according to the st_blksize of the first >> file it processes. We don't do this very often in the tree. I'm not &g

cat(1): always use a 64K buffer

2021-12-12 Thread Scott Cheloha
cat(1) sizes its I/O buffer according to the st_blksize of the first file it processes. We don't do this very often in the tree. I'm not sure if we should trust st_blksize. It would be simpler to just choose a value that works in practice and always use it. 64K works well. We settled on that

Re: kbind(2): push kernel lock down as needed

2021-12-08 Thread Scott Cheloha
> On Dec 8, 2021, at 12:24, Martin Pieuchot wrote: > > On 06/12/21(Mon) 14:58, Scott Cheloha wrote: >> On Mon, Dec 06, 2021 at 08:35:15PM +0100, Mark Kettenis wrote: >>>> Date: Sun, 5 Dec 2021 18:01:04 -0600 >>>> From: Scott Cheloha >>>> >

Re: Please test: UVM fault unlocking (aka vmobjlock)

2021-12-07 Thread Scott Cheloha
On Tue, Dec 07, 2021 at 10:04:04AM -0600, Scott Cheloha wrote: > On Mon, Nov 29, 2021 at 10:50:32PM +0100, Martin Pieuchot wrote: > > On 24/11/21(Wed) 11:16, Martin Pieuchot wrote: > > > Diff below unlock the bottom part of the UVM fault handler. I'm > > > interested

Re: Please test: UVM fault unlocking (aka vmobjlock)

2021-12-07 Thread Scott Cheloha
On Mon, Nov 29, 2021 at 10:50:32PM +0100, Martin Pieuchot wrote: > On 24/11/21(Wed) 11:16, Martin Pieuchot wrote: > > Diff below unlock the bottom part of the UVM fault handler. I'm > > interested in squashing the remaining bugs. Please test with your usual > > setup & report back. > > Thanks

Re: lsearch(3): implement with lfind(3)

2021-12-07 Thread Scott Cheloha
On Mon, Dec 06, 2021 at 10:37:02PM -0600, Scott Cheloha wrote: > lsearch(3) is just lfind(3) + append. If we write it that way we > shrink lsearch.c. The third function, linear_base(), is just added > complexity. The indirection buys us nothing. > > I don't think we need to keep

lsearch(3): implement with lfind(3)

2021-12-06 Thread Scott Cheloha
lsearch(3) is just lfind(3) + append. If we write it that way we shrink lsearch.c. The third function, linear_base(), is just added complexity. The indirection buys us nothing. I don't think we need to keep the historical comment, either. lsearch(3) is pure computation, it does not set errno:

uvn_reference(): correct printf() argument order

2021-12-06 Thread Scott Cheloha
The arguments to printf() here are backwards. ok? Index: uvm_vnode.c === RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v retrieving revision 1.119 diff -u -p -r1.119 uvm_vnode.c --- uvm_vnode.c 23 Oct 2021 14:42:08 - 1.119 +++

Re: lsearch(3): memmove(3), not memcpy(3)

2021-12-06 Thread Scott Cheloha
On Mon, Dec 06, 2021 at 03:09:05PM -0700, Theo de Raadt wrote: > +* Use memmove(3) instead of memcpy(3), just in case key > +* partially overlaps with the end of the array. > > It isn't a "just in case", as in a possibility. ? > It is gauranteed this condition will happen. I

Re: lsearch(3): memmove(3), not memcpy(3)

2021-12-06 Thread Scott Cheloha
On Mon, Dec 06, 2021 at 01:47:52PM -0700, Todd C. Miller wrote: > On Mon, 06 Dec 2021 09:03:43 -0600, Scott Cheloha wrote: > > > In lsearch(3), the key is allowed to overlap with the last element in > > base. We need to memmove(3), not memcpy(3), or we could corrupt the > &g

Re: kbind(2): push kernel lock down as needed

2021-12-06 Thread Scott Cheloha
On Mon, Dec 06, 2021 at 08:35:15PM +0100, Mark Kettenis wrote: > > Date: Sun, 5 Dec 2021 18:01:04 -0600 > > From: Scott Cheloha > > > > Two things in sys_kbind() need an explicit kernel lock. First, > > sigexit(). Second, uvm_map_extract(), because the following

moncontrol(3): remove hertz()

2021-12-06 Thread Scott Cheloha
In the moncontrol(3) code we have this fallback function, hertz(). The idea is to use an undocumented side effect of setitimer(2) to determine the width of the hardclock(9) tick. hertz() has a number of problems: 1. Per the setitimer(2) standard, the value of it_interval is ignored if

lsearch(3): memmove(3), not memcpy(3)

2021-12-06 Thread Scott Cheloha
In lsearch(3), the key is allowed to overlap with the last element in base. We need to memmove(3), not memcpy(3), or we could corrupt the key in that edge case. ok? Index: lsearch.c === RCS file:

kbind(2): push kernel lock down as needed

2021-12-05 Thread Scott Cheloha
Two things in sys_kbind() need an explicit kernel lock. First, sigexit(). Second, uvm_map_extract(), because the following call chain panics without it: panic assert uvn_reference uvm_mapent_clone uum_map_extract sys_kbind syscall Xsyscall uvn_reference() does KERNEL_ASSERT_LOCKED(). These

Re: tee(1): increase read buffer to MAXBSIZE bytes

2021-11-21 Thread Scott Cheloha
On Fri, Nov 19, 2021 at 08:40:37PM -0700, Theo de Raadt wrote: > Scott Cheloha wrote: > > > On Fri, Nov 19, 2021 at 07:42:18PM -0700, Theo de Raadt wrote: > > > +#include /* for MAXBSIZE */ > > > > > > No way, that is non-POSIX namespace. We are

Re: tee(1): increase read buffer to MAXBSIZE bytes

2021-11-20 Thread Scott Cheloha
On Sat, Nov 20, 2021 at 02:22:41PM -0700, Todd C. Miller wrote: > On Sat, 20 Nov 2021 11:19:13 -0600, Scott Cheloha wrote: > > > > One advantage to using stdio is that > > > it will use the optimal I/O blocksize automatically so you could > > > try using tha

Re: tee(1): increase read buffer to MAXBSIZE bytes

2021-11-20 Thread Scott Cheloha
On Fri, Nov 19, 2021 at 08:38:27PM -0700, Todd C. Miller wrote: > On Fri, 19 Nov 2021 21:31:12 -0600, Scott Cheloha wrote: > > > Is there a nicer way to pick a "reasonable" buffer size when we just > > want to move as many bytes as possible on a given platform wit

Re: tee(1): increase read buffer to MAXBSIZE bytes

2021-11-19 Thread Scott Cheloha
On Fri, Nov 19, 2021 at 07:42:18PM -0700, Theo de Raadt wrote: > +#include /* for MAXBSIZE */ > > No way, that is non-POSIX namespace. We are going in precisely the opposite > direction, eliminating this non-portability from the tree. > > No biblical scrolls have it written "all programs must

tee(1): increase read buffer to MAXBSIZE bytes

2021-11-19 Thread Scott Cheloha
Expanding the tee(1) read buffer trivially increases the throughput. In most applications the writer or the disk will be the bottleneck, but if tee(1) happens to be the bottleneck then the 8K buffer makes it worse. # dd(1) reading /dev/zero and writing to /dev/null $ for i in $(jot 10); do

nl(1): restart numbering at logical page start, not section start

2021-11-19 Thread Scott Cheloha
The manpage for nl(1) reads: The nl utility treats the text it reads in terms of logical pages. Unless specified otherwise, line numbering is reset at the start of each logical page. A logical page consists of a header, a body and a footer section; empty sections are valid. [...]

Re: Retry sleep in poll/select

2021-11-19 Thread Scott Cheloha
> On Nov 18, 2021, at 23:59, Sebastien Marie wrote: > > On Thu, Nov 18, 2021 at 07:50:01PM -0600, Scott Cheloha wrote: >>> On Thu, Nov 18, 2021 at 12:30:30PM +0100, Martin Pieuchot wrote: >>> On 17/11/21(Wed) 09:51, Scott Cheloha wrote: >>>>> On Nov

Re: Retry sleep in poll/select

2021-11-18 Thread Scott Cheloha
On Thu, Nov 18, 2021 at 12:30:30PM +0100, Martin Pieuchot wrote: > On 17/11/21(Wed) 09:51, Scott Cheloha wrote: > > > On Nov 17, 2021, at 03:22, Martin Pieuchot wrote: > > > > > > ???On 16/11/21(Tue) 13:55, Visa Hankala wrote: > > >> Currently,

Re: Retry sleep in poll/select

2021-11-17 Thread Scott Cheloha
> On Nov 17, 2021, at 03:22, Martin Pieuchot wrote: > > On 16/11/21(Tue) 13:55, Visa Hankala wrote: >> Currently, dopselect() and doppoll() call tsleep_nsec() without retry. >> cheloha@ asked if the functions should handle spurious wakeups. I guess >> such wakeups are unlikely with the nowake

wc(1): accelerate word counting

2021-11-17 Thread Scott Cheloha
In wc(1) we currently count words, both ASCII and multibyte, in a getline(3) loop. This makes sense in the multibyte case because stdio handles all the nasty buffer resizing for us. We avoid splitting a multibyte between two read(2) calls and the resulting code is simpler. However, for ASCII

Re: wc(1): fix NULL pointer dereference

2021-11-16 Thread Scott Cheloha
On Tue, Nov 16, 2021 at 12:39:32PM -0700, Todd C. Miller wrote: > On Tue, 16 Nov 2021 12:35:59 -0600, Scott Cheloha wrote: > > > In wc(1), the "file" argument to cnt() is NULL when we're enumerating > > the standard input. This causes a NULL pointer dereference if we

wc(1): fix NULL pointer dereference

2021-11-16 Thread Scott Cheloha
In wc(1), the "file" argument to cnt() is NULL when we're enumerating the standard input. This causes a NULL pointer dereference if we ever hit one of the warn(3) calls in that function. To fix, change the argument name to "path" and make "file" a local variable that is always initialized to

Re: sigsuspend(2): sleep on channel?

2021-11-11 Thread Scott Cheloha
On Thu, Nov 11, 2021 at 08:53:20PM +0100, Mark Kettenis wrote: > > Date: Thu, 11 Nov 2021 13:30:04 -0600 > > From: Scott Cheloha > > > > My understanding of sigsuspend(2) is that it only returns if a signal > > is delivered to the calling thread. However, in sys

sigsuspend(2): sleep on channel?

2021-11-11 Thread Scott Cheloha
My understanding of sigsuspend(2) is that it only returns if a signal is delivered to the calling thread. However, in sys_sigsuspend() we pass >p_p->ps_sigacts as the wakeup channel to tsleep_nsec(9). Are we actually waiting for a wakeup on that channel? Or can we sleep on here? Patch

Re: userret: take/release kernel lock once

2021-11-06 Thread Scott Cheloha
On Fri, Nov 05, 2021 at 03:15:26PM +0100, Christian Ludwig wrote: > > comments inline. > > [...] > > Unlocking if (!need_lock) below looks odd. I think it would make more > sense to reverse the logic: > > have_lock = 0; > > if (flags) { > if (!have_lock) { >

Re: userret: take/release kernel lock once

2021-11-04 Thread Scott Cheloha
On Thu, Nov 04, 2021 at 04:04:43PM -0500, Scott Cheloha wrote: > During userret() we can take/release the kernel lock up to four times. > Nobody actually uses SIGPROF or SIGVTALRM, but a double-lock with a > signal and a temporary signal mask is plausible. > > Is there any reaso

userret: take/release kernel lock once

2021-11-04 Thread Scott Cheloha
During userret() we can take/release the kernel lock up to four times. Nobody actually uses SIGPROF or SIGVTALRM, but a double-lock with a signal and a temporary signal mask is plausible. Is there any reason we can't take/release the kernel lock just once? Do we need to drop the kernel lock in

Re: uniq(1): ignore newline when comparing lines?

2021-11-02 Thread Scott Cheloha
On Tue, Nov 02, 2021 at 08:12:00AM -0600, Todd C. Miller wrote: > On Mon, 01 Nov 2021 21:04:54 -0500, Scott Cheloha wrote: > > > Yes it would be simpler. However I didn't want to start changing the > > input -- which we currently don't do -- without discussing it. > >

tr(1): next(): simplify NORMAL case

2021-11-01 Thread Scott Cheloha
The NORMAL case code in next() is needlessly complicated. The switch statement in particular is about as creative as it can possibly be. Isolate the early-return cases (end-of-string, bracket) from the could-be-a-range cases (backslash, any other character). Slightly less optimized, far simpler

Re: uniq(1): ignore newline when comparing lines?

2021-11-01 Thread Scott Cheloha
On Mon, Nov 01, 2021 at 07:33:58PM -0600, Todd C. Miller wrote: > On Mon, 01 Nov 2021 20:20:49 -0500, Scott Cheloha wrote: > > > On the one hand, it is intuitive that two buffers are not literally > > the same if one has a newline and the other does not. st

uniq(1): ignore newline when comparing lines?

2021-11-01 Thread Scott Cheloha
POSIX.1-2008 added a new sentence to the description of uniq: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/uniq.html > [...] The trailing of each line in the input shall be > ignored when doing comparisons. It comes from this interpretation:

Re: uniq(1): support arbitrarily long lines

2021-11-01 Thread Scott Cheloha
On Mon, Nov 01, 2021 at 10:27:40AM -0600, Todd C. Miller wrote: > On Mon, 01 Nov 2021 10:36:08 -0500, Scott Cheloha wrote: > > > My own testing here with pathological inputs didn't show that large of > > a performance difference between fgets(3) and getline(3). There wa

Re: uniq(1): support arbitrarily long lines

2021-11-01 Thread Scott Cheloha
On Mon, Nov 01, 2021 at 09:04:03AM +, Stuart Henderson wrote: > On 2021/10/31 20:48, Scott Cheloha wrote: > > In uniq(1), if we use getline(3) instead of fgets(3) we can support > > arbitrarily long lines. > > It works for me, and getting rid of the length restriction

uniq(1): support arbitrarily long lines

2021-10-31 Thread Scott Cheloha
In uniq(1), if we use getline(3) instead of fgets(3) we can support arbitrarily long lines. The only potentially confusing thing here is the pointer exchange within the loop. The current code uses fixed buffers so we just do a pointer swap. Easy. The new code uses dynamic buffers so we need to

tr(1): plug leak in genclass()

2021-10-31 Thread Scott Cheloha
In tr(1), if we have already generated a given character class we don't need to do it again. Further, we don't need to keep all the memory we allocate for the set of characters in the class. NCHARS + 1 is just an upper bound on the length. We should return whatever we don't need. This plugs a

tr(1): fix/cleanup backslash()

2021-10-31 Thread Scott Cheloha
Here is an attempt to clean up backslash() in tr(1). There are two real problems: - We should not treat '8' or '9' as being a part of an octal escape. They not octal digits. This yields some strange results: $ printf AB | tr AB '\78' | hexdump -c 000 @ @

Re: timecounting: use full 96-bit product when computing high-res time

2021-10-14 Thread Scott Cheloha
> On Oct 14, 2021, at 18:09, Scott Cheloha wrote: > > On Thu, Oct 14, 2021 at 11:37:44PM +0200, Mark Kettenis wrote: >>> Date: Thu, 14 Oct 2021 16:13:17 -0500 >>> From: Scott Cheloha >>> >>> [...] >>> >>> Thoughts? >>

Re: timecounting: use full 96-bit product when computing high-res time

2021-10-14 Thread Scott Cheloha
On Thu, Oct 14, 2021 at 11:37:44PM +0200, Mark Kettenis wrote: > > Date: Thu, 14 Oct 2021 16:13:17 -0500 > > From: Scott Cheloha > > > > [...] > > > > Thoughts? > > I never understood this code. But I don't understand that if our > timecounter

  1   2   3   4   5   6   7   >