timekeep: tk_generation problem

2020-07-12 Thread George Koehler
Hello tech list,

My CLOCK_MONOTONIC can jump backwards.  It looks like a problem with
tk_generation in the user timekeep page.  If tk_offset_count and
tk_offset change but tk_generation doesn't change, then libc can mix
old and new values and calculate a bogus time.

This diff tries to fix it.  The kernel has 2 sets of timehands, th0
and th1, but libc has only 1 timekeep page.  If the kernel switches
between th0 and th1 while they have the same generation, then libc
can't see the switch.  Is diff OK, or should we do something else?

The attached monocheck.c can detect the problem.
It loops until CLOCK_MONOTONIC decreases.--George

Index: kern/kern_tc.c
===
RCS file: /cvs/src/sys/kern/kern_tc.c,v
retrieving revision 1.62
diff -u -p -r1.62 kern_tc.c
--- kern/kern_tc.c  6 Jul 2020 13:33:09 -   1.62
+++ kern/kern_tc.c  13 Jul 2020 02:59:58 -
@@ -98,7 +98,8 @@ static struct timehands th0 = {
.th_counter = _timecounter,
.th_scale = UINT64_MAX / 100,
.th_offset = { .sec = 1, .frac = 0 },
-   .th_generation = 1,
+   /* Keep apart generations of th0, th1, for user timekeep. */
+   .th_generation = UINT_MAX / 2,
.th_next = 
 };
 
#include 
#include 
#include 

int
main(void)
{
struct timespec diff, now, then;

if (clock_gettime(CLOCK_MONOTONIC, ) == -1)
err(1, "clock_gettime");
for (;;) {
if (clock_gettime(CLOCK_MONOTONIC, ) == -1)
err(1, "clock_gettime");
if (timespeccmp(, , <)) {
timespecsub(, , );
errx(1, "then %lld.%09ld - now %lld.%09ld "
"= diff %lld.%09ld",
then.tv_sec, then.tv_nsec,
now.tv_sec,  now.tv_nsec,
diff.tv_sec, diff.tv_nsec);
}
then = now;
}
}


Re: softraid_crypto: add size to free call

2020-07-12 Thread Vitaliy Makkoveev
ok mvs@

> On 13 Jul 2020, at 01:22, Klemens Nanni  wrote:
> 
> On Sun, Jul 12, 2020 at 10:31:49PM +0300, Vitaliy Makkoveev wrote:
>> I like to have "sizeof(*omi)" in corresponding malloc(9) too.
>> 
>>  cut begin 
>> 827 omi = malloc(sizeof(struct sr_meta_opt_item), M_DEVBUF,
>> 828 M_WAITOK | M_ZERO);
>>  cut end 
> If you prefer to have malloc() and free() use the same idiom, I can
> commit the diff below, otherwise I'd refrain from changing existing code
> for this alone to avoid churn.
> 
> Feedback? OK?
> 
> Index: dev/softraid_crypto.c
> ===
> RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
> retrieving revision 1.138
> diff -u -p -r1.138 softraid_crypto.c
> --- dev/softraid_crypto.c 4 Jul 2019 18:09:17 -   1.138
> +++ dev/softraid_crypto.c 12 Jul 2020 22:21:09 -
> @@ -882,7 +882,7 @@ done:
>   for (omi = SLIST_FIRST(); omi != NULL; omi = omi_next) {
>   omi_next = SLIST_NEXT(omi, omi_link);
>   free(omi->omi_som, M_DEVBUF, 0);
> - free(omi, M_DEVBUF, 0);
> + free(omi, M_DEVBUF, sizeof(struct sr_meta_opt_item));
>   }
> 
>   free(sm, M_DEVBUF, SR_META_SIZE * DEV_BSIZE);
> 



Re: softraid_crypto: add size to free call

2020-07-12 Thread Klemens Nanni
On Sun, Jul 12, 2020 at 10:31:49PM +0300, Vitaliy Makkoveev wrote:
> I like to have "sizeof(*omi)" in corresponding malloc(9) too.
> 
>  cut begin 
> 827 omi = malloc(sizeof(struct sr_meta_opt_item), M_DEVBUF,
> 828 M_WAITOK | M_ZERO);
>  cut end 
If you prefer to have malloc() and free() use the same idiom, I can
commit the diff below, otherwise I'd refrain from changing existing code
for this alone to avoid churn.

Feedback? OK?

Index: dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.138
diff -u -p -r1.138 softraid_crypto.c
--- dev/softraid_crypto.c   4 Jul 2019 18:09:17 -   1.138
+++ dev/softraid_crypto.c   12 Jul 2020 22:21:09 -
@@ -882,7 +882,7 @@ done:
for (omi = SLIST_FIRST(); omi != NULL; omi = omi_next) {
omi_next = SLIST_NEXT(omi, omi_link);
free(omi->omi_som, M_DEVBUF, 0);
-   free(omi, M_DEVBUF, 0);
+   free(omi, M_DEVBUF, sizeof(struct sr_meta_opt_item));
}
 
free(sm, M_DEVBUF, SR_META_SIZE * DEV_BSIZE);



Re: softraid_crypto: add size to free call

2020-07-12 Thread Vitaliy Makkoveev
On Sun, Jul 12, 2020 at 08:51:08PM +0200, Klemens Nanni wrote:
> While omi->omi_som seems variable in size, omi is only ever allocated
> with one size and softraid.c uses the same size for free(9) as well.
> 
> Tested with cryto softraid and keydisk.
> 
> Feedback? OK?
> 
> 
> Index: dev/softraid_crypto.c
> ===
> RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
> retrieving revision 1.138
> diff -u -p -r1.138 softraid_crypto.c
> --- dev/softraid_crypto.c 4 Jul 2019 18:09:17 -   1.138
> +++ dev/softraid_crypto.c 12 Jul 2020 18:00:29 -
> @@ -882,7 +882,7 @@ done:
>   for (omi = SLIST_FIRST(); omi != NULL; omi = omi_next) {
>   omi_next = SLIST_NEXT(omi, omi_link);
>   free(omi->omi_som, M_DEVBUF, 0);
> - free(omi, M_DEVBUF, 0);
> + free(omi, M_DEVBUF, sizeof(*omi));
>   }
>  
>   free(sm, M_DEVBUF, SR_META_SIZE * DEV_BSIZE);
> 

I like to have "sizeof(*omi)" in corresponding malloc(9) too.

 cut begin 
827 omi = malloc(sizeof(struct sr_meta_opt_item), M_DEVBUF,
828 M_WAITOK | M_ZERO);
 cut end 



softraid_crypto: add size to free call

2020-07-12 Thread Klemens Nanni
While omi->omi_som seems variable in size, omi is only ever allocated
with one size and softraid.c uses the same size for free(9) as well.

Tested with cryto softraid and keydisk.

Feedback? OK?


Index: dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.138
diff -u -p -r1.138 softraid_crypto.c
--- dev/softraid_crypto.c   4 Jul 2019 18:09:17 -   1.138
+++ dev/softraid_crypto.c   12 Jul 2020 18:00:29 -
@@ -882,7 +882,7 @@ done:
for (omi = SLIST_FIRST(); omi != NULL; omi = omi_next) {
omi_next = SLIST_NEXT(omi, omi_link);
free(omi->omi_som, M_DEVBUF, 0);
-   free(omi, M_DEVBUF, 0);
+   free(omi, M_DEVBUF, sizeof(*omi));
}
 
free(sm, M_DEVBUF, SR_META_SIZE * DEV_BSIZE);



Re: wg: fix build without pf

2020-07-12 Thread Vitaliy Makkoveev
On Sun, Jul 12, 2020 at 07:44:47PM +0200, Klemens Nanni wrote:

OK mvs@

> Feedback? OK?
> 
> 
> Index: sys/net/if_wg.c
> ===
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 if_wg.c
> --- sys/net/if_wg.c   10 Jul 2020 13:26:42 -  1.9
> +++ sys/net/if_wg.c   12 Jul 2020 16:31:03 -
> @@ -1666,7 +1666,9 @@ wg_decap(struct wg_softc *sc, struct mbu
>   m->m_pkthdr.ph_ifidx = sc->sc_if.if_index;
>   m->m_pkthdr.ph_rtableid = sc->sc_if.if_rdomain;
>   m->m_flags &= ~(M_MCAST | M_BCAST);
> +#if NPF > 0
>   pf_pkt_addr_changed(m);
> +#endif /* NPF > 0 */
>  
>  done:
>   t->t_mbuf = m;
> 



softraid: fix malloc size

2020-07-12 Thread Klemens Nanni
We overallocate by quite a bit for bootable disks;  spotted while
reading the code and tested by installboot(8)ing onto vnd(4) backed
softraid disks (after booting a kernel with this diff).

$ egdb --quiet obj/bsd.gdb
Reading symbols from obj/bsd.gdb...done.
(gdb) p sizeof(struct sr_meta_crypto)
$1 = 2480
(gdb) p sizeof(struct sr_meta_boot)
$2 = 168

Feedback? OK?

Index: sys/dev/softraid.c
===
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.405
diff -u -p -r1.405 softraid.c
--- sys/dev/softraid.c  27 Jun 2020 17:28:58 -  1.405
+++ sys/dev/softraid.c  12 Jul 2020 17:59:56 -
@@ -3736,7 +3736,7 @@ sr_ioctl_installboot(struct sr_softc *sc
if (omi == NULL) {
omi = malloc(sizeof(struct sr_meta_opt_item), M_DEVBUF,
M_WAITOK | M_ZERO);
-   omi->omi_som = malloc(sizeof(struct sr_meta_crypto), M_DEVBUF,
+   omi->omi_som = malloc(sizeof(struct sr_meta_boot), M_DEVBUF,
M_WAITOK | M_ZERO);
omi->omi_som->som_type = SR_OPT_BOOT;
omi->omi_som->som_length = sizeof(struct sr_meta_boot);



Re: fix build without pf

2020-07-12 Thread Jason A. Donenfeld
ok zx2c4



Re: powerpc(64): tweak timecounter mask

2020-07-12 Thread Mark Kettenis
> Date: Sun, 12 Jul 2020 19:30:38 +0200
> From: Christian Weisgerber 
> 
> Mark Kettenis:
> 
> > > Date: Sun, 12 Jul 2020 18:12:39 +0200
> > > From: Christian Weisgerber 
> > > 
> > > The PowerPC/Power ISA Time Base is a 64-bit register.  We can use
> > > the full lower 32 bits.
> > > 
> > > OK?
> > 
> > Sure, but this needs to be coordinated with the userland diff.
> 
> No.  tc_update_timekeep() copies the counter mask into the timekeep
> structure and the userland picks it up from there.

Ah right, good!



Re: armv7: tweak timercounter mask

2020-07-12 Thread Christian Weisgerber
Mark Kettenis:

> > There is the strong suspicion that the 0x7fff mask in the various
> > armv7 timecounters was simply copied from powerpc, and that these really
> > are full 32-bit counters.
> > 
> > I wanted to verify this from the data sheets, but I'm insufficiently
> > familiar with the ARM ecosystem to locate those.
> 
> The counter is described in the ARM Architecture Reference Manual.  It
> was introduced later so you need to look at revision C or later.

Found it.  That's agtimer.

amptimer is the global timer in the ARM Cortex-A9 MPCore Technical
Reference Manual.  It's a 64-bit counter.

For gptimer, I've found the OMAP4460 Technical Reference Manual.
It's a 32-bit counter.

So they should be all fine with 0x.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



wg: fix build without pf

2020-07-12 Thread Klemens Nanni
Feedback? OK?


Index: sys/net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.9
diff -u -p -r1.9 if_wg.c
--- sys/net/if_wg.c 10 Jul 2020 13:26:42 -  1.9
+++ sys/net/if_wg.c 12 Jul 2020 16:31:03 -
@@ -1666,7 +1666,9 @@ wg_decap(struct wg_softc *sc, struct mbu
m->m_pkthdr.ph_ifidx = sc->sc_if.if_index;
m->m_pkthdr.ph_rtableid = sc->sc_if.if_rdomain;
m->m_flags &= ~(M_MCAST | M_BCAST);
+#if NPF > 0
pf_pkt_addr_changed(m);
+#endif /* NPF > 0 */
 
 done:
t->t_mbuf = m;



Re: powerpc(64): tweak timecounter mask

2020-07-12 Thread Christian Weisgerber
Mark Kettenis:

> > Date: Sun, 12 Jul 2020 18:12:39 +0200
> > From: Christian Weisgerber 
> > 
> > The PowerPC/Power ISA Time Base is a 64-bit register.  We can use
> > the full lower 32 bits.
> > 
> > OK?
> 
> Sure, but this needs to be coordinated with the userland diff.

No.  tc_update_timekeep() copies the counter mask into the timekeep
structure and the userland picks it up from there.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: [PATCH} Optimized rasops32 putchar

2020-07-12 Thread Frederic Cambus
On Fri, Jun 26, 2020 at 07:42:50AM -0700, jo...@armadilloaerospace.com wrote:
> Optimized 32 bit character rendering with unrolled rows and pairwise
> foreground / background pixel rendering.
> 
> If it weren't for the 5x8 font, I would have just assumed everything
> was an even width and made the fallback path also pairwise.
> 
> In isolation, the 16x32 character case got 2x faster, but that wasn't
> a huge real world speedup where the space rendering that was already
> at memory bandwidth limits accounted for most of the character
> rendering time.  However, in combination with the previous fast
> conditional console scrolling that removes most of the space rendering,
> it becomes significant.

On my Ryzen desktop with radeondrm, I don't see any improvements, the
rasops_vcons_copyrows() optimizations seems to have made character
plotting fast enough so that it's not a bottleneck anymore, which is
definitely great.

cpu0: AMD Ryzen 7 2700 Eight-Core Processor, 3394.18 MHz, 17-08-02
radeondrm0 at pci8 dev 0 function 0 "ATI Radeon HD 6450" rev 0x00
radeondrm0: 1920x1080, 32bpp

On my T450 however, this diff makes cat'ing my usual test file [1] up
to 20% faster with the default 12x24 font on the built-in 1600x900
screen, which I think is significant enough for the diff to go in.

cpu0: Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz, 2095.47 MHz, 06-3d-04
inteldrm0 at pci0 dev 2 function 0 "Intel HD Graphics 5500" rev 0x09
drm0 at inteldrm0
inteldrm0: 1600x900, 32bpp

On my Cubieboard2 (armv7) I didn't notice any meaningful difference,
which I assume is to be expected on a 32-bit platform. I suppose it's
also reasonable to assume other 32-bit platforms (i386, hppa, macppc)
will not see any regression beyond noise level?
 
Anyone willing to OK this diff?

[1] https://norvig.com/big.txt



Re: powerpc(64): tweak timecounter mask

2020-07-12 Thread Mark Kettenis
> Date: Sun, 12 Jul 2020 18:23:09 +0200
> From: Christian Weisgerber 
> 
> Christian Weisgerber:
> 
> > -   tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL, 0
> > +   tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL, 0
> 
> PS: Do we prefer ~0u over 0x?

I prefer 0x.  We typically use upper-case U for unsigned
constants so ~1u looks really weird to me.



Re: powerpc(64): tweak timecounter mask

2020-07-12 Thread Mark Kettenis
> Date: Sun, 12 Jul 2020 18:12:39 +0200
> From: Christian Weisgerber 
> 
> The PowerPC/Power ISA Time Base is a 64-bit register.  We can use
> the full lower 32 bits.
> 
> OK?

Sure, but this needs to be coordinated with the userland diff.  And
we'd better change it quick because doing it later is an ABI break.

> Index: arch/macppc/macppc/clock.c
> ===
> RCS file: /cvs/src/sys/arch/macppc/macppc/clock.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 clock.c
> --- arch/macppc/macppc/clock.c6 Jul 2020 13:33:08 -   1.44
> +++ arch/macppc/macppc/clock.c12 Jul 2020 15:17:48 -
> @@ -57,7 +57,7 @@ u_int32_t ns_per_tick = 320;
>  static int32_t ticks_per_intr;
>  
>  static struct timecounter tb_timecounter = {
> - tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL, 0
> + tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL, 0
>  };
>  
>  /* calibrate the timecounter frequency for the listed models */
> Index: arch/powerpc64/powerpc64/clock.c
> ===
> RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/clock.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 clock.c
> --- arch/powerpc64/powerpc64/clock.c  10 Jun 2020 19:06:53 -  1.1
> +++ arch/powerpc64/powerpc64/clock.c  12 Jul 2020 15:18:02 -
> @@ -37,7 +37,7 @@ struct evcount stat_count;
>  u_inttb_get_timecount(struct timecounter *);
>  
>  static struct timecounter tb_timecounter = {
> - tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL
> + tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL
>  };
>  
>  void cpu_startclock(void);
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 
> 



Re: armv7: tweak timercounter mask

2020-07-12 Thread Mark Kettenis
> Date: Sun, 12 Jul 2020 18:21:48 +0200
> From: Christian Weisgerber 
> 
> There is the strong suspicion that the 0x7fff mask in the various
> armv7 timecounters was simply copied from powerpc, and that these really
> are full 32-bit counters.
> 
> I wanted to verify this from the data sheets, but I'm insufficiently
> familiar with the ARM ecosystem to locate those.

The counter is described in the ARM Architecture Reference Manual.  It
was introduced later so you need to look at revision C or later.

Anyway, the counter is at least 56-bits wide.  Given that we're making
the same change to arm64, this is ok kettenis@.

> Index: arch/arm/cortex/agtimer.c
> ===
> RCS file: /cvs/src/sys/arch/arm/cortex/agtimer.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 agtimer.c
> --- arch/arm/cortex/agtimer.c 11 Aug 2018 10:42:42 -  1.9
> +++ arch/arm/cortex/agtimer.c 12 Jul 2020 16:13:22 -
> @@ -46,7 +46,7 @@ int32_t agtimer_frequency = TIMER_FREQUE
>  u_int agtimer_get_timecount(struct timecounter *);
>  
>  static struct timecounter agtimer_timecounter = {
> - agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL
> + agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL
>  };
>  
>  struct agtimer_pcpu_softc {
> Index: arch/arm/cortex/amptimer.c
> ===
> RCS file: /cvs/src/sys/arch/arm/cortex/amptimer.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 amptimer.c
> --- arch/arm/cortex/amptimer.c6 Jul 2020 13:33:06 -   1.7
> +++ arch/arm/cortex/amptimer.c12 Jul 2020 16:13:37 -
> @@ -67,7 +67,7 @@ int32_t amptimer_frequency = TIMER_FREQU
>  u_int amptimer_get_timecount(struct timecounter *);
>  
>  static struct timecounter amptimer_timecounter = {
> - amptimer_get_timecount, NULL, 0x7fff, 0, "amptimer", 0, NULL, 0
> + amptimer_get_timecount, NULL, 0x, 0, "amptimer", 0, NULL, 0
>  };
>  
>  #define MAX_ARM_CPUS 8
> Index: arch/armv7/omap/gptimer.c
> ===
> RCS file: /cvs/src/sys/arch/armv7/omap/gptimer.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 gptimer.c
> --- arch/armv7/omap/gptimer.c 6 Jul 2020 13:33:07 -   1.8
> +++ arch/armv7/omap/gptimer.c 12 Jul 2020 15:53:06 -
> @@ -117,7 +117,7 @@ int gptimer_irq = 0;
>  u_int gptimer_get_timecount(struct timecounter *);
>  
>  static struct timecounter gptimer_timecounter = {
> - gptimer_get_timecount, NULL, 0x7fff, 0, "gptimer", 0, NULL, 0
> + gptimer_get_timecount, NULL, 0x, 0, "gptimer", 0, NULL, 0
>  };
>  
>  volatile u_int32_t nexttickevent;
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 
> 



armv7: tweak timercounter mask

2020-07-12 Thread Christian Weisgerber
There is the strong suspicion that the 0x7fff mask in the various
armv7 timecounters was simply copied from powerpc, and that these really
are full 32-bit counters.

I wanted to verify this from the data sheets, but I'm insufficiently
familiar with the ARM ecosystem to locate those.

Back in September 2017, Artturi Alm proposed the very same change
here but failed to make himself heard.


Index: arch/arm/cortex/agtimer.c
===
RCS file: /cvs/src/sys/arch/arm/cortex/agtimer.c,v
retrieving revision 1.9
diff -u -p -r1.9 agtimer.c
--- arch/arm/cortex/agtimer.c   11 Aug 2018 10:42:42 -  1.9
+++ arch/arm/cortex/agtimer.c   12 Jul 2020 16:13:22 -
@@ -46,7 +46,7 @@ int32_t agtimer_frequency = TIMER_FREQUE
 u_int agtimer_get_timecount(struct timecounter *);
 
 static struct timecounter agtimer_timecounter = {
-   agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL
+   agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL
 };
 
 struct agtimer_pcpu_softc {
Index: arch/arm/cortex/amptimer.c
===
RCS file: /cvs/src/sys/arch/arm/cortex/amptimer.c,v
retrieving revision 1.7
diff -u -p -r1.7 amptimer.c
--- arch/arm/cortex/amptimer.c  6 Jul 2020 13:33:06 -   1.7
+++ arch/arm/cortex/amptimer.c  12 Jul 2020 16:13:37 -
@@ -67,7 +67,7 @@ int32_t amptimer_frequency = TIMER_FREQU
 u_int amptimer_get_timecount(struct timecounter *);
 
 static struct timecounter amptimer_timecounter = {
-   amptimer_get_timecount, NULL, 0x7fff, 0, "amptimer", 0, NULL, 0
+   amptimer_get_timecount, NULL, 0x, 0, "amptimer", 0, NULL, 0
 };
 
 #define MAX_ARM_CPUS   8
Index: arch/armv7/omap/gptimer.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/gptimer.c,v
retrieving revision 1.8
diff -u -p -r1.8 gptimer.c
--- arch/armv7/omap/gptimer.c   6 Jul 2020 13:33:07 -   1.8
+++ arch/armv7/omap/gptimer.c   12 Jul 2020 15:53:06 -
@@ -117,7 +117,7 @@ int gptimer_irq = 0;
 u_int gptimer_get_timecount(struct timecounter *);
 
 static struct timecounter gptimer_timecounter = {
-   gptimer_get_timecount, NULL, 0x7fff, 0, "gptimer", 0, NULL, 0
+   gptimer_get_timecount, NULL, 0x, 0, "gptimer", 0, NULL, 0
 };
 
 volatile u_int32_t nexttickevent;
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: powerpc(64): tweak timecounter mask

2020-07-12 Thread Christian Weisgerber
Christian Weisgerber:

> - tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL, 0
> + tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL, 0

PS: Do we prefer ~0u over 0x?

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



powerpc(64): tweak timecounter mask

2020-07-12 Thread Christian Weisgerber
The PowerPC/Power ISA Time Base is a 64-bit register.  We can use
the full lower 32 bits.

OK?

Index: arch/macppc/macppc/clock.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/clock.c,v
retrieving revision 1.44
diff -u -p -r1.44 clock.c
--- arch/macppc/macppc/clock.c  6 Jul 2020 13:33:08 -   1.44
+++ arch/macppc/macppc/clock.c  12 Jul 2020 15:17:48 -
@@ -57,7 +57,7 @@ u_int32_t ns_per_tick = 320;
 static int32_t ticks_per_intr;
 
 static struct timecounter tb_timecounter = {
-   tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL, 0
+   tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL, 0
 };
 
 /* calibrate the timecounter frequency for the listed models */
Index: arch/powerpc64/powerpc64/clock.c
===
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/clock.c,v
retrieving revision 1.1
diff -u -p -r1.1 clock.c
--- arch/powerpc64/powerpc64/clock.c10 Jun 2020 19:06:53 -  1.1
+++ arch/powerpc64/powerpc64/clock.c12 Jul 2020 15:18:02 -
@@ -37,7 +37,7 @@ struct evcount stat_count;
 u_int  tb_get_timecount(struct timecounter *);
 
 static struct timecounter tb_timecounter = {
-   tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL
+   tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL
 };
 
 void   cpu_startclock(void);
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: Undefined Behavior at jsmn.c

2020-07-12 Thread Ali Farzanrad
Florian Obser wrote:
> On Sun, Jul 12, 2020 at 09:10:57AM +0200, Otto Moerbeek wrote:
> > On Sun, Jul 12, 2020 at 09:57:02AM +0430, Ali Farzanrad wrote:
> > 
> > > Hi @tech,
> > > 
> > > I was comparing jsmn.c in acme-client with jsmn.c in FreeBSD [1].
> > > I found a switch without a default case which is an undefined behavior:
> > > 
> > > @@ -69,6 +69,8 @@
> > >   case '\t' : case '\r' : case '\n' : case ' ' :
> > >   case ','  : case ']'  : case '}' :
> > >   goto found;
> > > + default:
> > > + break;
> > >   }
> > >   if (js[parser->pos] < 32 || js[parser->pos] >= 127) {
> > >   parser->pos = start;
> > > 
> > > I have patched that undefined behavior + some style fix.
> > 
> > It is bad practise to intermix style changes with bug fixes. 
> > Please post the fix seperately.
> 
> It's also not undefined behaviour.
> ISO/IEC 9899:1999 6.8.4.2:
> 
>   If no converted case constant expression matches and there is no
>   default label, no part of the switch body is executed.

My bad, sorry.

> Not having a default case might be considered bad style in itself
> though.

I also compared it to latest jsmn code, and found out that they have
fixed few other things, like preventing array / object as a key in
another object.

Is it OK to sync in-base jsmn with upstream?  It seems harmless (only
stricter + few style).

(I've attached a single diff, because it is simple sync)


cvs diff: Diffing usr.sbin/acme-client
Index: usr.sbin/acme-client/jsmn.c
===
RCS file: /cvs/src/usr.sbin/acme-client/jsmn.c,v
retrieving revision 1.1
diff -u -p -r1.1 jsmn.c
--- usr.sbin/acme-client/jsmn.c 31 Aug 2016 22:01:42 -  1.1
+++ usr.sbin/acme-client/jsmn.c 12 Jul 2020 14:20:06 -
@@ -1,31 +1,34 @@
 /*
- Copyright (c) 2010 Serge A. Zaitsev
- 
- Permission is hereby granted, free of charge, to any person obtaining a copy
- of this software and associated documentation files (the "Software"), to deal
- in the Software without restriction, including without limitation the rights
- to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- copies of the Software, and to permit persons to whom the Software is
- furnished to do so, subject to the following conditions:
- 
- The above copyright notice and this permission notice shall be included in
- all copies or substantial portions of the Software.
- 
- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- THE SOFTWARE.*
+ * MIT License
+ *
+ * Copyright (c) 2010 Serge Zaitsev
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
THE
+ * SOFTWARE.
  */
+
 #include "jsmn.h"
 
 /**
- * Allocates a fresh unused token from the token pull.
+ * Allocates a fresh unused token from the token pool.
  */
 static jsmntok_t *jsmn_alloc_token(jsmn_parser *parser,
-   jsmntok_t *tokens, size_t num_tokens) {
+   jsmntok_t *tokens, const size_t num_tokens) {
jsmntok_t *tok;
if (parser->toknext >= num_tokens) {
return NULL;
@@ -42,8 +45,8 @@ static jsmntok_t *jsmn_alloc_token(jsmn_
 /**
  * Fills token type and boundaries.
  */
-static void jsmn_fill_token(jsmntok_t *token, jsmntype_t type,
-int start, int end) {
+static void jsmn_fill_token(jsmntok_t *token, const jsmntype_t type,
+   const int start, const int end) {
token->type = type;
token->start = start;

Re: fsck_ffs: faster with lots of cylinder groups

2020-07-12 Thread Solene Rapenne
On Sun, 12 Jul 2020 09:13:47 +0200
Otto Moerbeek :

> On Mon, Jun 29, 2020 at 02:30:41PM +0200, Otto Moerbeek wrote:
> 
> > On Sun, Jun 21, 2020 at 03:35:21PM +0200, Otto Moerbeek wrote:
> >   
> > > Hi,
> > > 
> > > both phase 1 and phase 5 need cylinder group metadata.  This diff
> > > keeps the cg data read in phase 1 in memory to be used by phase 5 if
> > > possible. From FreeBSD. 
> > > 
> > >   -Otto
> > > 
> > > On an empty 30T fileystem:
> > > 
> > > $ time obj/fsck_ffs -f /dev/sd3a
> > > 2m44.10s real 0m13.21s user 0m07.38s system
> > > 
> > > $ time doas obj/fsck_ffs -f /dev/sd3a
> > > 1m32.81s real 0m12.86s user 0m05.25s system
> > > 
> > > The difference will be less if a fileystem is filled up, but still nice.  
> > 
> > Any takers?  
> 
> No feedback. I'm getting discouraged in doing more filesystem work...
> 
> What to do?
> 
> 1) Abondon the diff
> 2) Commit without ok
> 
> I did quite extensive testing, but both options are unsatisfactory.
> 
>   -Otto

I'm not sure how to test your diff.
Would running fsck on a sane filesystem enough?

Are you using Vms that you halt to force a
fsck on them? Would this be a good test too?



macppc G5 pmap fix

2020-07-12 Thread Mark Kettenis
While working on the OpenBSD/powerpc64 pmap I noticed that the code we
use for the G5 machines has a bug and doesn't remove execute
permission from mappings when it should.

Since I don't have a G5 machine readily available, can somebody test
this diff for me?


Index: arch/powerpc/powerpc/pmap.c
===
RCS file: /cvs/src/sys/arch/powerpc/powerpc/pmap.c,v
retrieving revision 1.172
diff -u -p -r1.172 pmap.c
--- arch/powerpc/powerpc/pmap.c 15 Apr 2020 08:09:00 -  1.172
+++ arch/powerpc/powerpc/pmap.c 12 Jul 2020 09:30:51 -
@@ -2005,8 +2005,7 @@ pmap_pted_ro64(struct pte_desc *pted, vm
}
 
/* Add a Page Table Entry, section 7.6.3.1. */
-   ptp64->pte_lo &= ~(PTE_CHG_64|PTE_PP_64);
-   ptp64->pte_lo |= PTE_RO_64;
+   ptp64->pte_lo = pted->p.pted_pte64.pte_lo;
eieio();/* Order 1st PTE update before 2nd. */
ptp64->pte_hi |= PTE_VALID_64;
sync(); /* Ensure updates completed. */



Re: fsck_ffs: faster with lots of cylinder groups

2020-07-12 Thread Otto Moerbeek
On Sun, Jul 12, 2020 at 11:07:05AM +0200, Solene Rapenne wrote:

> On Sun, 12 Jul 2020 09:13:47 +0200
> Otto Moerbeek :
> 
> > On Mon, Jun 29, 2020 at 02:30:41PM +0200, Otto Moerbeek wrote:
> > 
> > > On Sun, Jun 21, 2020 at 03:35:21PM +0200, Otto Moerbeek wrote:
> > >   
> > > > Hi,
> > > > 
> > > > both phase 1 and phase 5 need cylinder group metadata.  This diff
> > > > keeps the cg data read in phase 1 in memory to be used by phase 5 if
> > > > possible. From FreeBSD. 
> > > > 
> > > > -Otto
> > > > 
> > > > On an empty 30T fileystem:
> > > > 
> > > > $ time obj/fsck_ffs -f /dev/sd3a
> > > > 2m44.10s real 0m13.21s user 0m07.38s system
> > > > 
> > > > $ time doas obj/fsck_ffs -f /dev/sd3a
> > > > 1m32.81s real 0m12.86s user 0m05.25s system
> > > > 
> > > > The difference will be less if a fileystem is filled up, but still 
> > > > nice.  
> > > 
> > > Any takers?  
> > 
> > No feedback. I'm getting discouraged in doing more filesystem work...
> > 
> > What to do?
> > 
> > 1) Abondon the diff
> > 2) Commit without ok
> > 
> > I did quite extensive testing, but both options are unsatisfactory.
> > 
> > -Otto
> 
> I'm not sure how to test your diff.
> Would running fsck on a sane filesystem enough?
> 
> Are you using Vms that you halt to force a
> fsck on them? Would this be a good test too?

I have used both large and small fieysystems, clean and with
inconsistencies, both ffs1 and ffs2. Sometimes I create
inconsistencies by power cycling a machine, buut I have created faulty
filesystems by carefully overwriting meta data with dd in the past as
well.

In this case running with a restricted ulimit -d to force the fallback
code to kick in is also an good idea.

-Otto



Re: arm64 usertc

2020-07-12 Thread Mark Kettenis
> Date: Sun, 12 Jul 2020 00:06:21 +0200
> From: Christian Weisgerber 
> 
> Mark Kettenis:
> 
> > Nevertheless, here is a different take on the problem. Since the
> > timecounter only uses the low 32 bits we don't need the double read.
> > This version also changes the timecounter mask from 0x7fff to
> > 0x.  That must be ok, since the counter has 64 bits and we are
> > already using 0x as a mask on amd64 and sparc64.
> > 
> > ok?
> 
> Yes, but don't forget the part in sys/arch/arm64/include/timetc.h.
> 
> Also, if I may ask, ...
> 
> > Index: sys/arch/arm64/dev/agtimer.c
> > ===
> > RCS file: /cvs/src/sys/arch/arm64/dev/agtimer.c,v
> > retrieving revision 1.14
> > diff -u -p -r1.14 agtimer.c
> > --- sys/arch/arm64/dev/agtimer.c11 Jul 2020 15:22:44 -  1.14
> > +++ sys/arch/arm64/dev/agtimer.c11 Jul 2020 18:35:12 -
> > @@ -43,7 +43,8 @@ int32_t agtimer_frequency = TIMER_FREQUE
> >  u_int agtimer_get_timecount(struct timecounter *);
> >  
> >  static struct timecounter agtimer_timecounter = {
> > -   agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL, 0
> > +   agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL,
> > +   TC_AGTIMER
> >  };
> >  
> >  struct agtimer_pcpu_softc {
> > @@ -191,7 +192,15 @@ agtimer_attach(struct device *parent, st
> >  u_int
> >  agtimer_get_timecount(struct timecounter *tc)
> >  {
> > -   return agtimer_readcnt64();
> > +   uint64_t val;
> > +
> > +   /*
> > +* No need to work around Cortex-A73 errata 858921 since we
> > +* only look at the low 32 bits here.
> > +*/
> > +   __asm volatile("isb" ::: "memory");
> > +   __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
> > +   return (val & 0x);
> 
> Is there any point, stylistically I mean, to explicitly masking
> this over the truncation implicit in the types?  I'm pretty sure
> we do, say, "intvar = int64var" all the time.

I tried to make it explicit that the result is truncated to 32 bits.
But indeed this isn't necessary.  The compiler knows and optimizes it
away completely.  I can leave it out if you think that's better.

> 
> >  }
> >  
> >  int
> > Index: lib/libc/arch/aarch64/gen/usertc.c
> > ===
> > RCS file: /cvs/src/lib/libc/arch/aarch64/gen/usertc.c,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 usertc.c
> > --- lib/libc/arch/aarch64/gen/usertc.c  6 Jul 2020 13:33:05 -   
> > 1.1
> > +++ lib/libc/arch/aarch64/gen/usertc.c  11 Jul 2020 18:35:12 -
> > @@ -1,6 +1,6 @@
> > -/* $OpenBSD: usertc.c,v 1.1 2020/07/06 13:33:05 pirofti Exp $  */
> > +/* $OpenBSD$   */
> >  /*
> > - * Copyright (c) 2020 Paul Irofti 
> > + * Copyright (c) 2020 Mark Kettenis 
> >   *
> >   * Permission to use, copy, modify, and distribute this software for any
> >   * purpose with or without fee is hereby granted, provided that the above
> > @@ -18,4 +18,30 @@
> >  #include 
> >  #include 
> >  
> > -int (*const _tc_get_timecount)(struct timekeep *, u_int *) = NULL;
> > +static inline u_int
> > +agtimer_get_timecount(struct timecounter *tc)
> > +{
> > +   uint64_t val;
> > +
> > +   /*
> > +* No need to work around Cortex-A73 errata 858921 since we
> > +* only look at the low 32 bits here.
> > +*/
> > +   __asm volatile("isb" ::: "memory");
> > +   __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
> > +   return (val & 0x);
> > +}
> > +
> > +static int
> > +tc_get_timecount(struct timekeep *tk, u_int *tc)
> > +{
> > +   switch (tk->tk_user) {
> > +   case TC_AGTIMER:
> > +   *tc = agtimer_get_timecount(NULL);
> > +   return 0;
> > +   }
> > +
> > +   return -1;
> > +}
> > +
> > +int (*const _tc_get_timecount)(struct timekeep *, u_int *) = 
> > tc_get_timecount;
> > 
> 
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 



Re: Undefined Behavior at jsmn.c

2020-07-12 Thread Florian Obser
On Sun, Jul 12, 2020 at 09:10:57AM +0200, Otto Moerbeek wrote:
> On Sun, Jul 12, 2020 at 09:57:02AM +0430, Ali Farzanrad wrote:
> 
> > Hi @tech,
> > 
> > I was comparing jsmn.c in acme-client with jsmn.c in FreeBSD [1].
> > I found a switch without a default case which is an undefined behavior:
> > 
> > @@ -69,6 +69,8 @@
> > case '\t' : case '\r' : case '\n' : case ' ' :
> > case ','  : case ']'  : case '}' :
> > goto found;
> > +   default:
> > +   break;
> > }
> > if (js[parser->pos] < 32 || js[parser->pos] >= 127) {
> > parser->pos = start;
> > 
> > I have patched that undefined behavior + some style fix.
> 
> It is bad practise to intermix style changes with bug fixes. 
> Please post the fix seperately.

It's also not undefined behaviour.
ISO/IEC 9899:1999 6.8.4.2:

If no converted case constant expression matches and there is no
default label, no part of the switch body is executed.

Not having a default case might be considered bad style in itself
though.

> 
>   -Otto
> 

-- 
I'm not entirely sure you are real.



Re: fsck_ffs: faster with lots of cylinder groups

2020-07-12 Thread Otto Moerbeek
On Mon, Jun 29, 2020 at 02:30:41PM +0200, Otto Moerbeek wrote:

> On Sun, Jun 21, 2020 at 03:35:21PM +0200, Otto Moerbeek wrote:
> 
> > Hi,
> > 
> > both phase 1 and phase 5 need cylinder group metadata.  This diff
> > keeps the cg data read in phase 1 in memory to be used by phase 5 if
> > possible. From FreeBSD. 
> > 
> > -Otto
> > 
> > On an empty 30T fileystem:
> > 
> > $ time obj/fsck_ffs -f /dev/sd3a
> > 2m44.10s real 0m13.21s user 0m07.38s system
> > 
> > $ time doas obj/fsck_ffs -f /dev/sd3a
> > 1m32.81s real 0m12.86s user 0m05.25s system
> > 
> > The difference will be less if a fileystem is filled up, but still nice.
> 
> Any takers?

No feedback. I'm getting discouraged in doing more filesystem work...

What to do?

1) Abondon the diff
2) Commit without ok

I did quite extensive testing, but both options are unsatisfactory.

-Otto

> 
> > 
> > Index: fsck.h
> > ===
> > RCS file: /cvs/src/sbin/fsck_ffs/fsck.h,v
> > retrieving revision 1.32
> > diff -u -p -r1.32 fsck.h
> > --- fsck.h  5 Jan 2018 09:33:47 -   1.32
> > +++ fsck.h  21 Jun 2020 12:48:50 -
> > @@ -136,7 +136,6 @@ struct bufarea {
> >  struct bufarea bufhead;/* head of list of other blks in 
> > filesys */
> >  struct bufarea sblk;   /* file system superblock */
> >  struct bufarea asblk;  /* alternate file system superblock */
> > -struct bufarea cgblk;  /* cylinder group blocks */
> >  struct bufarea *pdirbp;/* current directory contents */
> >  struct bufarea *pbp;   /* current inode block */
> >  struct bufarea *getdatablk(daddr_t, long);
> > @@ -148,9 +147,7 @@ struct bufarea *getdatablk(daddr_t, long
> > (bp)->b_flags = 0;
> >  
> >  #definesbdirty()   sblk.b_dirty = 1
> > -#definecgdirty()   cgblk.b_dirty = 1
> >  #definesblock  (*sblk.b_un.b_fs)
> > -#definecgrp(*cgblk.b_un.b_cg)
> >  
> >  enum fixstate {DONTKNOW, NOFIX, FIX, IGNORE};
> >  
> > @@ -275,9 +272,13 @@ struct ufs2_dinode ufs2_zino;
> >  #defineFOUND   0x10
> >  
> >  union dinode *ginode(ino_t);
> > +struct bufarea *cglookup(u_int cg);
> >  struct inoinfo *getinoinfo(ino_t);
> >  void getblk(struct bufarea *, daddr_t, long);
> >  ino_t allocino(ino_t, int);
> > +void *Malloc(size_t);
> > +void *Calloc(size_t, size_t);
> > +void *Reallocarray(void *, size_t, size_t);
> >  
> >  int(*info_fn)(char *, size_t);
> >  char   *info_filesys;
> > Index: inode.c
> > ===
> > RCS file: /cvs/src/sbin/fsck_ffs/inode.c,v
> > retrieving revision 1.49
> > diff -u -p -r1.49 inode.c
> > --- inode.c 16 Sep 2018 02:43:11 -  1.49
> > +++ inode.c 21 Jun 2020 12:48:50 -
> > @@ -370,7 +370,7 @@ setinodebuf(ino_t inum)
> > partialsize = inobufsize;
> > }
> > if (inodebuf == NULL &&
> > -   (inodebuf = malloc((unsigned)inobufsize)) == NULL)
> > +   (inodebuf = Malloc((unsigned)inobufsize)) == NULL)
> > errexit("Cannot allocate space for inode buffer\n");
> >  }
> >  
> > @@ -401,7 +401,7 @@ cacheino(union dinode *dp, ino_t inumber
> > blks = howmany(DIP(dp, di_size), sblock.fs_bsize);
> > if (blks > NDADDR)
> > blks = NDADDR + NIADDR;
> > -   inp = malloc(sizeof(*inp) + (blks ? blks - 1 : 0) * sizeof(daddr_t));
> > +   inp = Malloc(sizeof(*inp) + (blks ? blks - 1 : 0) * sizeof(daddr_t));
> > if (inp == NULL)
> > errexit("cannot allocate memory for inode cache\n");
> > inpp = [inumber % numdirs];
> > @@ -423,10 +423,10 @@ cacheino(union dinode *dp, ino_t inumber
> > inp->i_blks[NDADDR + i] = DIP(dp, di_ib[i]);
> > if (inplast == listmax) {
> > newlistmax = listmax + 100;
> > -   newinpsort = reallocarray(inpsort,
> > +   newinpsort = Reallocarray(inpsort,
> > (unsigned)newlistmax, sizeof(struct inoinfo *));
> > if (newinpsort == NULL)
> > -   errexit("cannot increase directory list");
> > +   errexit("cannot increase directory list\n");
> > inpsort = newinpsort;
> > listmax = newlistmax;
> > }
> > @@ -582,7 +582,8 @@ allocino(ino_t request, int type)
> >  {
> > ino_t ino;
> > union dinode *dp;
> > -   struct cg *cgp = 
> > +   struct bufarea *cgbp;
> > +   struct cg *cgp;
> > int cg;
> > time_t t;
> > struct inostat *info;
> > @@ -602,7 +603,7 @@ allocino(ino_t request, int type)
> > unsigned long newalloced, i;
> > newalloced = MINIMUM(sblock.fs_ipg,
> > MAXIMUM(2 * inostathead[cg].il_numalloced, 10));
> > -   info = calloc(newalloced, sizeof(struct inostat));
> > +   info = Calloc(newalloced, sizeof(struct inostat));
> > if (info == NULL) {
> > 

Re: Undefined Behavior at jsmn.c

2020-07-12 Thread Otto Moerbeek
On Sun, Jul 12, 2020 at 09:57:02AM +0430, Ali Farzanrad wrote:

> Hi @tech,
> 
> I was comparing jsmn.c in acme-client with jsmn.c in FreeBSD [1].
> I found a switch without a default case which is an undefined behavior:
> 
> @@ -69,6 +69,8 @@
>   case '\t' : case '\r' : case '\n' : case ' ' :
>   case ','  : case ']'  : case '}' :
>   goto found;
> + default:
> + break;
>   }
>   if (js[parser->pos] < 32 || js[parser->pos] >= 127) {
>   parser->pos = start;
> 
> I have patched that undefined behavior + some style fix.

It is bad practise to intermix style changes with bug fixes. 
Please post the fix seperately.

-Otto

> 
> [1] https://svnweb.freebsd.org/base/head/lib/libpmc/pmu-events/jsmn.c
> 
> Index: jsmn.c
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/jsmn.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 jsmn.c
> --- jsmn.c31 Aug 2016 22:01:42 -  1.1
> +++ jsmn.c12 Jul 2020 05:10:34 -
> @@ -1,31 +1,33 @@
>  /*
> - Copyright (c) 2010 Serge A. Zaitsev
> - 
> - Permission is hereby granted, free of charge, to any person obtaining a copy
> - of this software and associated documentation files (the "Software"), to 
> deal
> - in the Software without restriction, including without limitation the rights
> - to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - copies of the Software, and to permit persons to whom the Software is
> - furnished to do so, subject to the following conditions:
> - 
> - The above copyright notice and this permission notice shall be included in
> - all copies or substantial portions of the Software.
> - 
> - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> - AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> - OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - THE SOFTWARE.*
> + * Copyright (c) 2010 Serge A. Zaitsev
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
>   */
> +
>  #include "jsmn.h"
>  
> -/**
> - * Allocates a fresh unused token from the token pull.
> +/*
> + * Allocates a fresh unused token from the token pool.
>   */
> -static jsmntok_t *jsmn_alloc_token(jsmn_parser *parser,
> - jsmntok_t *tokens, size_t num_tokens) {
> +static jsmntok_t *
> +jsmn_alloc_token(jsmn_parser *parser, jsmntok_t *tokens, size_t num_tokens)
> +{
>   jsmntok_t *tok;
>   if (parser->toknext >= num_tokens) {
>   return NULL;
> @@ -39,22 +41,25 @@ static jsmntok_t *jsmn_alloc_token(jsmn_
>   return tok;
>  }
>  
> -/**
> +/*
>   * Fills token type and boundaries.
>   */
> -static void jsmn_fill_token(jsmntok_t *token, jsmntype_t type,
> -int start, int end) {
> +static void
> +jsmn_fill_token(jsmntok_t *token, jsmntype_t type, int start, int end)
> +{
>   token->type = type;
>   token->start = start;
>   token->end = end;
>   token->size = 0;
>  }
>  
> -/**
> +/*
>   * Fills next available token with JSON primitive.
>   */
> -static int jsmn_parse_primitive(jsmn_parser *parser, const char *js,
> - size_t len, jsmntok_t *tokens, size_t num_tokens) {
> +static int
> +jsmn_parse_primitive(jsmn_parser *parser, const char *js,
> +size_t len, jsmntok_t *tokens, size_t num_tokens)
> +{
>   jsmntok_t *token;
>   int start;
>  
> @@ -63,12 +68,19 @@ static int jsmn_parse_primitive(jsmn_par
>   for (; parser->pos < len &&