Re: PATCH: speed up pkg_add

2022-03-15 Thread Stuart Henderson
On 2022/03/15 11:50, Marc Espie wrote:
> I've changed pkg_add for updates and tied files a while back, which resulted
> in creating a lot of temp files. The logic was complicated, and I didn't
> want to make it more complex at the time until most of the issues were fixed.
> 
> Then we got tags, so that most packing-lists are innocuous these days.
> 
> Now is that time, the following patch may speed up some updates and additions
> significantly, in case the packing-lists don't contain "random execs".
> 
> What this does:
> - in case we extract a new file that's not on the filesystem, we can extract
> in-place instead of going through a temporary item
> 
> - in case we are matching files with identical files during an update, and
> the filename doesn't change, we acknowledge we don't need to do anything.
> 
> Caveat: this may bite. It is advised to run pkg_check afterwards and
> report file system discrepancies.

I've gone through updates on 3 machines (I ran pkg_check before and
after), no problems seen and it is faster for me.



Re: Swap encrypt under memory pressure

2022-03-15 Thread Stuart Henderson
On 2022/03/12 11:35, Martin Pieuchot wrote:
> Try to allocate the buffer before doing the encryption, if it fails we
> do not spend time doing the encryption.  This reduce the pressure when
> swapping with low memory.

Done a couple of ports bulk builds with this on i386, no problems seen.



Re: xlocale/ctypes again

2022-03-15 Thread Mark Kettenis
> Date: Tue, 15 Mar 2022 20:59:07 +
> From: Stuart Henderson 
> 
> Rather out of my depth but I ran into another instance of the problem
> that's come up before with ctypes/C++ headers/xlocale (see also
> https://marc.info/?l=openbsd-bugs=157758838031146=2 and
> https://marc.info/?l=openbsd-bugs=161528248931449=2), i.e.
> the C++ headers unconditionally depend on something which is
> only defined conditionally.
> 
> In this case it was with a port which wanted to use _XOPEN_SOURCE=600,
> so __BSD_VISIBLE is defined to 0, so the definition of vasprintf() in
> stdio.h is hidden, but it's used here:
> 
> /usr/include/c++/v1/__bsd_locale_fallbacks.h:
> 117 inline
> 118 int __libcpp_asprintf_l(char **__s, locale_t __l, const char *__format
> , ...) {
> 119 va_list __va;
> 120 va_start(__va, __format);
> 121 __libcpp_locale_guard __current(__l);
> 122 int __res = vasprintf(__s, __format, __va);
> 123 va_end(__va);
> 124 return __res;
> 125 }
> 
> Unfortunately for this one it's not just providing a definition in
> the locale header but is actually used in code:
> 
> /usr/include/c++/v1/locale:
>1566 template 
>1567 _OutputIterator
>1568 num_put<_CharT, _OutputIterator>::do_put(iter_type __s, ios_base& __io
> b,
>1569  char_type __fl, double __v) c
> onst
>1570 {
> ...
>1585 if (__nc > static_cast(__nbuf-1))
>1586 {
>1587 if (__specify_precision)
>1588 __nc = __libcpp_asprintf_l(&__nb, _LIBCPP_GET_C_LOCALE, 
> __fmt, (int)__iob.precision(), __v);
>1589 else
>1590 __nc = __libcpp_asprintf_l(&__nb, _LIBCPP_GET_C_LOCALE, 
> __fmt, __v);
>1591 if (__nc == -1)
>1592 __throw_bad_alloc();
>1593 __nbh.reset(__nb);
> 
> As well as the vasprintf it also hit the usual "unknown type name
> 'locale_t'" for the various functions in __strtonum_fallback.h.
> 
> For the port concerned (games/gargoyle) patching to remove _XOPEN_SOURCE
> was enough to workaround but that's not always the case (and it's rather
> non-obvious from the errors).
> 
> I tried forward-porting jsg's diff in the earlier thread, but the files
> changed a bit in the meantime with libc++ updates, and it seems the
> locale_t-related #ifdefs miss many of the entries in the file anyway
> - and adding them to the other cases is going to make the header
> rather non-functional.
> 
> Does anyone have ideas about how we could handle this? Or should we
> just ignore for now and continue to throw rocks at ports where we run
> into it? ;)

Implement  like FreeBSD has.  It is not difficult since our
locale support is pretty much nonexistent.  It is just work, and a lot
of testing.



xlocale/ctypes again

2022-03-15 Thread Stuart Henderson
Rather out of my depth but I ran into another instance of the problem
that's come up before with ctypes/C++ headers/xlocale (see also
https://marc.info/?l=openbsd-bugs=157758838031146=2 and
https://marc.info/?l=openbsd-bugs=161528248931449=2), i.e.
the C++ headers unconditionally depend on something which is
only defined conditionally.

In this case it was with a port which wanted to use _XOPEN_SOURCE=600,
so __BSD_VISIBLE is defined to 0, so the definition of vasprintf() in
stdio.h is hidden, but it's used here:

/usr/include/c++/v1/__bsd_locale_fallbacks.h:
117 inline
118 int __libcpp_asprintf_l(char **__s, locale_t __l, const char *__format
, ...) {
119 va_list __va;
120 va_start(__va, __format);
121 __libcpp_locale_guard __current(__l);
122 int __res = vasprintf(__s, __format, __va);
123 va_end(__va);
124 return __res;
125 }

Unfortunately for this one it's not just providing a definition in
the locale header but is actually used in code:

/usr/include/c++/v1/locale:
   1566 template 
   1567 _OutputIterator
   1568 num_put<_CharT, _OutputIterator>::do_put(iter_type __s, ios_base& __io
b,
   1569  char_type __fl, double __v) c
onst
   1570 {
...
   1585 if (__nc > static_cast(__nbuf-1))
   1586 {
   1587 if (__specify_precision)
   1588 __nc = __libcpp_asprintf_l(&__nb, _LIBCPP_GET_C_LOCALE, 
__fmt, (int)__iob.precision(), __v);
   1589 else
   1590 __nc = __libcpp_asprintf_l(&__nb, _LIBCPP_GET_C_LOCALE, 
__fmt, __v);
   1591 if (__nc == -1)
   1592 __throw_bad_alloc();
   1593 __nbh.reset(__nb);

As well as the vasprintf it also hit the usual "unknown type name
'locale_t'" for the various functions in __strtonum_fallback.h.

For the port concerned (games/gargoyle) patching to remove _XOPEN_SOURCE
was enough to workaround but that's not always the case (and it's rather
non-obvious from the errors).

I tried forward-porting jsg's diff in the earlier thread, but the files
changed a bit in the meantime with libc++ updates, and it seems the
locale_t-related #ifdefs miss many of the entries in the file anyway
- and adding them to the other cases is going to make the header
rather non-functional.

Does anyone have ideas about how we could handle this? Or should we
just ignore for now and continue to throw rocks at ports where we run
into it? ;)



OpenBSD Errata: March 15, 2022 (libcrypto)

2022-03-15 Thread Alexander Bluhm
Errata patches for libcrypto bignum have been released for OpenBSD
6.9 and 7.0.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata69.html
  https://www.openbsd.org/errata70.html



Re: bgpd mark EoR prefix with a flag field

2022-03-15 Thread Theo Buehler
On Tue, Mar 15, 2022 at 04:03:20PM +0100, Claudio Jeker wrote:
> Currently EoR markers use a full byte in struct prefix what can be done in
> a bit. Use the last flags field so that that 1 byte is available again.
> I already have a need for that byte this is why I came up with this
> change.

ok



Re: bgpd mark EoR prefix with a flag field

2022-03-15 Thread Denis Fondras
Le Tue, Mar 15, 2022 at 04:03:20PM +0100, Claudio Jeker a écrit :
> Currently EoR markers use a full byte in struct prefix what can be done in
> a bit. Use the last flags field so that that 1 byte is available again.
> I already have a need for that byte this is why I came up with this
> change.
>  

OK denis@

> -- 
> :wq Claudio
> 
> ? obj
> Index: rde.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
> retrieving revision 1.247
> diff -u -p -r1.247 rde.h
> --- rde.h 2 Mar 2022 16:51:43 -   1.247
> +++ rde.h 15 Mar 2022 14:59:27 -
> @@ -333,7 +333,7 @@ struct prefix {
>   uint32_t path_id_tx;
>   uint8_t  validation_state;
>   uint8_t  nhflags;
> - uint8_t  eor;
> + uint8_t  unused;
>   uint8_t  flags;
>  #define  PREFIX_FLAG_WITHDRAW0x01/* enqueued on withdraw queue */
>  #define  PREFIX_FLAG_UPDATE  0x02/* enqueued on update queue */
> @@ -341,6 +341,7 @@ struct prefix {
>  #define  PREFIX_FLAG_STALE   0x08/* stale entry (graceful 
> reload) */
>  #define  PREFIX_FLAG_MASK0x0f/* mask for the prefix types */
>  #define  PREFIX_FLAG_ADJOUT  0x10/* prefix is in the adj-out rib 
> */
> +#define  PREFIX_FLAG_EOR 0x20/* prefix is EoR */
>  #define  PREFIX_NEXTHOP_LINKED   0x40/* prefix is linked onto 
> nexthop list */
>  #define  PREFIX_FLAG_LOCKED  0x80/* locked by rib walker */
>  };
> Index: rde_rib.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.233
> diff -u -p -r1.233 rde_rib.c
> --- rde_rib.c 15 Mar 2022 14:39:34 -  1.233
> +++ rde_rib.c 15 Mar 2022 14:59:28 -
> @@ -875,10 +875,10 @@ prefix_index_cmp(struct prefix *a, struc
>  static inline int
>  prefix_cmp(struct prefix *a, struct prefix *b)
>  {
> - if (a->eor != b->eor)
> - return a->eor - b->eor;
> - /* if EOR marker no need to check the rest also a->eor == b->eor */
> - if (a->eor)
> + if ((a->flags & PREFIX_FLAG_EOR) != (b->flags & PREFIX_FLAG_EOR))
> + return (a->flags & PREFIX_FLAG_EOR) ? 1 : -1;
> + /* if EOR marker no need to check the rest */
> + if (a->flags & PREFIX_FLAG_EOR)
>   return 0;
>  
>   if (a->aspath != b->aspath)
> @@ -1152,8 +1152,7 @@ prefix_add_eor(struct rde_peer *peer, ui
>   struct prefix *p;
>  
>   p = prefix_alloc();
> - p->flags = PREFIX_FLAG_ADJOUT | PREFIX_FLAG_UPDATE;
> - p->eor = 1;
> + p->flags = PREFIX_FLAG_ADJOUT | PREFIX_FLAG_UPDATE | PREFIX_FLAG_EOR;
>   if (RB_INSERT(prefix_tree, >updates[aid], p) != NULL)
>   /* no need to add if EoR marker already present */
>   prefix_free(p);
> @@ -1290,7 +1289,7 @@ prefix_adjout_destroy(struct prefix *p)
>   if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
>   fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
>  
> - if (p->eor) {
> + if (p->flags & PREFIX_FLAG_EOR) {
>   /* EOR marker is not linked in the index */
>   prefix_free(p);
>   return;
> Index: rde_update.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
> retrieving revision 1.136
> diff -u -p -r1.136 rde_update.c
> --- rde_update.c  2 Mar 2022 16:51:43 -   1.136
> +++ rde_update.c  15 Mar 2022 14:59:28 -
> @@ -586,7 +586,7 @@ up_is_eor(struct rde_peer *peer, uint8_t
>   struct prefix *p;
>  
>   p = RB_MIN(prefix_tree, >updates[aid]);
> - if (p != NULL && p->eor) {
> + if (p != NULL && (p->flags & PREFIX_FLAG_EOR)) {
>   /*
>* Need to remove eor from update tree because
>* prefix_adjout_destroy() can't handle that.
> @@ -635,7 +635,7 @@ up_dump_prefix(u_char *buf, int len, str
>   np->communities != p->communities ||
>   np->nexthop != p->nexthop ||
>   np->nhflags != p->nhflags ||
> - np->eor)
> + (np->flags & PREFIX_FLAG_EOR))
>   done = 1;
>  
>  
> 



bgpd mark EoR prefix with a flag field

2022-03-15 Thread Claudio Jeker
Currently EoR markers use a full byte in struct prefix what can be done in
a bit. Use the last flags field so that that 1 byte is available again.
I already have a need for that byte this is why I came up with this
change.
 
-- 
:wq Claudio

? obj
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.247
diff -u -p -r1.247 rde.h
--- rde.h   2 Mar 2022 16:51:43 -   1.247
+++ rde.h   15 Mar 2022 14:59:27 -
@@ -333,7 +333,7 @@ struct prefix {
uint32_t path_id_tx;
uint8_t  validation_state;
uint8_t  nhflags;
-   uint8_t  eor;
+   uint8_t  unused;
uint8_t  flags;
 #definePREFIX_FLAG_WITHDRAW0x01/* enqueued on withdraw queue */
 #definePREFIX_FLAG_UPDATE  0x02/* enqueued on update queue */
@@ -341,6 +341,7 @@ struct prefix {
 #definePREFIX_FLAG_STALE   0x08/* stale entry (graceful 
reload) */
 #definePREFIX_FLAG_MASK0x0f/* mask for the prefix types */
 #definePREFIX_FLAG_ADJOUT  0x10/* prefix is in the adj-out rib 
*/
+#definePREFIX_FLAG_EOR 0x20/* prefix is EoR */
 #definePREFIX_NEXTHOP_LINKED   0x40/* prefix is linked onto 
nexthop list */
 #definePREFIX_FLAG_LOCKED  0x80/* locked by rib walker */
 };
Index: rde_rib.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
retrieving revision 1.233
diff -u -p -r1.233 rde_rib.c
--- rde_rib.c   15 Mar 2022 14:39:34 -  1.233
+++ rde_rib.c   15 Mar 2022 14:59:28 -
@@ -875,10 +875,10 @@ prefix_index_cmp(struct prefix *a, struc
 static inline int
 prefix_cmp(struct prefix *a, struct prefix *b)
 {
-   if (a->eor != b->eor)
-   return a->eor - b->eor;
-   /* if EOR marker no need to check the rest also a->eor == b->eor */
-   if (a->eor)
+   if ((a->flags & PREFIX_FLAG_EOR) != (b->flags & PREFIX_FLAG_EOR))
+   return (a->flags & PREFIX_FLAG_EOR) ? 1 : -1;
+   /* if EOR marker no need to check the rest */
+   if (a->flags & PREFIX_FLAG_EOR)
return 0;
 
if (a->aspath != b->aspath)
@@ -1152,8 +1152,7 @@ prefix_add_eor(struct rde_peer *peer, ui
struct prefix *p;
 
p = prefix_alloc();
-   p->flags = PREFIX_FLAG_ADJOUT | PREFIX_FLAG_UPDATE;
-   p->eor = 1;
+   p->flags = PREFIX_FLAG_ADJOUT | PREFIX_FLAG_UPDATE | PREFIX_FLAG_EOR;
if (RB_INSERT(prefix_tree, >updates[aid], p) != NULL)
/* no need to add if EoR marker already present */
prefix_free(p);
@@ -1290,7 +1289,7 @@ prefix_adjout_destroy(struct prefix *p)
if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
 
-   if (p->eor) {
+   if (p->flags & PREFIX_FLAG_EOR) {
/* EOR marker is not linked in the index */
prefix_free(p);
return;
Index: rde_update.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
retrieving revision 1.136
diff -u -p -r1.136 rde_update.c
--- rde_update.c2 Mar 2022 16:51:43 -   1.136
+++ rde_update.c15 Mar 2022 14:59:28 -
@@ -586,7 +586,7 @@ up_is_eor(struct rde_peer *peer, uint8_t
struct prefix *p;
 
p = RB_MIN(prefix_tree, >updates[aid]);
-   if (p != NULL && p->eor) {
+   if (p != NULL && (p->flags & PREFIX_FLAG_EOR)) {
/*
 * Need to remove eor from update tree because
 * prefix_adjout_destroy() can't handle that.
@@ -635,7 +635,7 @@ up_dump_prefix(u_char *buf, int len, str
np->communities != p->communities ||
np->nexthop != p->nexthop ||
np->nhflags != p->nhflags ||
-   np->eor)
+   (np->flags & PREFIX_FLAG_EOR))
done = 1;
 
 



Use refcnt API in kqueue

2022-03-15 Thread Visa Hankala
Make kqueue use the refcnt API.

OK?

Index: sys/kern/kern_event.c
===
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.183
diff -u -p -r1.183 kern_event.c
--- sys/kern/kern_event.c   22 Feb 2022 01:15:01 -  1.183
+++ sys/kern/kern_event.c   15 Mar 2022 13:43:20 -
@@ -199,7 +199,7 @@ const struct filterops *const sysfilt_op
 void
 KQREF(struct kqueue *kq)
 {
-   atomic_inc_int(>kq_refs);
+   refcnt_take(>kq_refcnt);
 }
 
 void
@@ -207,7 +207,7 @@ KQRELE(struct kqueue *kq)
 {
struct filedesc *fdp;
 
-   if (atomic_dec_int_nv(>kq_refs) > 0)
+   if (refcnt_rele(>kq_refcnt) == 0)
return;
 
fdp = kq->kq_fdp;
@@ -837,7 +837,7 @@ kqpoll_exit(void)
 
kqueue_purge(p, p->p_kq);
kqueue_terminate(p, p->p_kq);
-   KASSERT(p->p_kq->kq_refs == 1);
+   KASSERT(p->p_kq->kq_refcnt.r_refs == 1);
KQRELE(p->p_kq);
p->p_kq = NULL;
 }
@@ -848,7 +848,7 @@ kqueue_alloc(struct filedesc *fdp)
struct kqueue *kq;
 
kq = pool_get(_pool, PR_WAITOK | PR_ZERO);
-   kq->kq_refs = 1;
+   refcnt_init(>kq_refcnt);
kq->kq_fdp = fdp;
TAILQ_INIT(>kq_head);
mtx_init(>kq_lock, IPL_HIGH);
Index: sys/sys/eventvar.h
===
RCS file: src/sys/sys/eventvar.h,v
retrieving revision 1.13
diff -u -p -r1.13 eventvar.h
--- sys/sys/eventvar.h  8 Feb 2022 08:56:41 -   1.13
+++ sys/sys/eventvar.h  15 Mar 2022 13:43:20 -
@@ -32,6 +32,7 @@
 #define _SYS_EVENTVAR_H_
 
 #include 
+#include 
 #include 
 
 #define KQ_NEVENTS 8   /* minimize copy{in,out} calls */
@@ -47,7 +48,7 @@ struct kqueue {
struct  mutex kq_lock;  /* lock for queue access */
TAILQ_HEAD(, knote) kq_head;/* [q] list of pending event */
int kq_count;   /* [q] # of pending events */
-   u_int   kq_refs;/* [a] # of references */
+   struct  refcnt kq_refcnt;   /* [a] # of references */
struct  selinfo kq_sel;
struct  filedesc *kq_fdp;   /* [I] fd table of this kq */
 



Re: Add refcnt_read()

2022-03-15 Thread Visa Hankala
On Tue, Mar 15, 2022 at 09:11:30AM +, Visa Hankala wrote:
> This patch adds a function for getting a snapshot of a reference
> counter. This will let code like crcopy() access the value in an
> API-observing way.

Here is a revised version.

Based on input from dlg@, the patch now adds refcnt_shared() for
testing if the object has multiple references. This interface is
possibly more robust than refcnt_read().

The patch still provides refcnt_read() for special (mis)uses.

OK?

Index: share/man/man9/refcnt_init.9
===
RCS file: src/share/man/man9/refcnt_init.9,v
retrieving revision 1.1
diff -u -p -r1.1 refcnt_init.9
--- share/man/man9/refcnt_init.911 Sep 2015 19:13:22 -  1.1
+++ share/man/man9/refcnt_init.915 Mar 2022 13:43:20 -
@@ -23,6 +23,8 @@
 .Nm refcnt_rele ,
 .Nm refcnt_rele_wake ,
 .Nm refcnt_finalize ,
+.Nm refcnt_shared ,
+.Nm refcnt_read ,
 .Nm REFCNT_INITIALIZER
 .Nd reference count API
 .Sh SYNOPSIS
@@ -37,6 +39,10 @@
 .Fn "refcnt_rele_wake" "struct refcnt *r"
 .Ft void
 .Fn "refcnt_finalize" "struct refcnt *r" "const char *wmesg"
+.Ft int
+.Fn "refcnt_shared" "struct refcnt *r"
+.Ft unsigned int
+.Fn "refcnt_read" "struct refcnt *r"
 .Fn "REFCNT_INITIALIZER"
 .Sh DESCRIPTION
 The refcnt API provides simple reference counters that can be used
@@ -68,14 +74,26 @@ There may only be one caller to
 per refcnt
 .Fa r .
 .Pp
+.Fn refcnt_shared
+tests if the object has multiple references.
+.Pp
+.Fn refcnt_read
+returns a snapshot of the counter value.
+Its use is discouraged,
+code should use
+.Fn refcnt_shared
+whenever possible.
+.Pp
 .Fn REFCNT_INITIALIZER
 initialises a declaration of a refcnt to 1.
 .Sh CONTEXT
 .Fn refcnt_init ,
 .Fn refcnt_take ,
 .Fn refcnt_rele ,
+.Fn refcnt_rele_wake ,
+.Fn refcnt_shared
 and
-.Fn refcnt_rele_wake
+.Fn refcnt_read
 can be called during autoconf, from process context, or from interrupt
 context.
 .Pp
@@ -85,3 +103,10 @@ can be called from process context.
 .Fn refcnt_rele
 returns a non-zero value if the last reference has been released,
 otherwise 0.
+.Pp
+.Fn refcnt_shared
+returns a non-zero value if the object has multiple references,
+otherwise 0.
+.Pp
+.Fn refcnt_read
+returns a snapshot of the counter value.
Index: sys/kern/kern_synch.c
===
RCS file: src/sys/kern/kern_synch.c,v
retrieving revision 1.183
diff -u -p -r1.183 kern_synch.c
--- sys/kern/kern_synch.c   10 Mar 2022 15:21:08 -  1.183
+++ sys/kern/kern_synch.c   15 Mar 2022 13:43:20 -
@@ -852,6 +852,18 @@ refcnt_finalize(struct refcnt *r, const 
}
 }
 
+int
+refcnt_shared(struct refcnt *r)
+{
+   return (atomic_load_int(>r_refs) > 1);
+}
+
+unsigned int
+refcnt_read(struct refcnt *r)
+{
+   return (atomic_load_int(>r_refs));
+}
+
 void
 cond_init(struct cond *c)
 {
Index: sys/sys/refcnt.h
===
RCS file: src/sys/sys/refcnt.h,v
retrieving revision 1.5
diff -u -p -r1.5 refcnt.h
--- sys/sys/refcnt.h10 Mar 2022 15:21:08 -  1.5
+++ sys/sys/refcnt.h15 Mar 2022 13:43:20 -
@@ -37,6 +37,8 @@ void  refcnt_take(struct refcnt *);
 intrefcnt_rele(struct refcnt *);
 void   refcnt_rele_wake(struct refcnt *);
 void   refcnt_finalize(struct refcnt *, const char *);
+intrefcnt_shared(struct refcnt *);
+unsigned int   refcnt_read(struct refcnt *);
 
 #endif /* _KERNEL */
 



Re: bgpd refactor prefix_adjout_update

2022-03-15 Thread Theo Buehler
On Tue, Mar 15, 2022 at 12:21:00PM +0100, Claudio Jeker wrote:
> This diff just refactors the code by moving the alloc part up.
> It makes the code a bit easier to read and more similar with other
> prefix_adjout functions. Also I plan to pass the struct prefix in
> as an argument and do the prefix_adjout_get() in the callee.

ok

> @@ -1222,6 +1186,40 @@ prefix_adjout_update(struct rde_peer *pe
>   if (RB_INSERT(prefix_index, >adj_rib_out, p) != NULL)
>   fatalx("%s: RB index invariant violated", __func__);
>   }
> +
> + if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
> + fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
> + if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) {
> + if (prefix_nhflags(p) == state->nhflags &&
> + prefix_nexthop(p) == state->nexthop &&
> + communities_equal(>communities,
> + prefix_communities(p)) &&
> + path_compare(>aspath, prefix_aspath(p)) ==
> + 0) {

0) { fits on the previous line.



bgpd refactor prefix_adjout_update

2022-03-15 Thread Claudio Jeker
This diff just refactors the code by moving the alloc part up.
It makes the code a bit easier to read and more similar with other
prefix_adjout functions. Also I plan to pass the struct prefix in
as an argument and do the prefix_adjout_get() in the callee.

-- 
:wq Claudio

Index: rde_rib.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
retrieving revision 1.232
diff -u -p -r1.232 rde_rib.c
--- rde_rib.c   2 Mar 2022 16:51:43 -   1.232
+++ rde_rib.c   2 Mar 2022 17:39:12 -
@@ -1171,46 +1171,10 @@ prefix_adjout_update(struct rde_peer *pe
struct rde_community *comm;
struct prefix *p;
 
-   if ((p = prefix_adjout_get(peer, 0, prefix, prefixlen)) != NULL) {
-   if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
-   fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit",
-   __func__);
-
-   /* prefix is already in the Adj-RIB-Out */
-   if (p->flags & PREFIX_FLAG_WITHDRAW) {
-   RB_REMOVE(prefix_tree,
-   >withdraws[prefix->aid], p);
-   peer->up_wcnt--;
-   }
-   if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) ==
-   0) {
-   if (prefix_nhflags(p) == state->nhflags &&
-   prefix_nexthop(p) == state->nexthop &&
-   communities_equal(>communities,
-   prefix_communities(p)) &&
-   path_compare(>aspath, prefix_aspath(p)) ==
-   0) {
-   /* nothing changed */
-   p->validation_state = vstate;
-   p->lastchange = getmonotime();
-   p->flags &= ~PREFIX_FLAG_STALE;
-   return;
-   }
-
-   if (p->flags & PREFIX_FLAG_UPDATE) {
-   RB_REMOVE(prefix_tree,
-   >updates[prefix->aid], p);
-   peer->up_nlricnt--;
-   }
-   /* unlink prefix so it can be relinked below */
-   prefix_unlink(p);
-   peer->prefix_out_cnt--;
-   }
-   /* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */
-   p->flags &= ~PREFIX_FLAG_MASK;
-   } else {
+   if ((p = prefix_adjout_get(peer, 0, prefix, prefixlen)) == NULL) {
p = prefix_alloc();
-   p->flags |= PREFIX_FLAG_ADJOUT;
+   /* initally mark DEAD so code below is skipped */
+   p->flags |= PREFIX_FLAG_ADJOUT | PREFIX_FLAG_DEAD;
 
p->pt = pt_get(prefix, prefixlen);
if (p->pt == NULL)
@@ -1222,6 +1186,40 @@ prefix_adjout_update(struct rde_peer *pe
if (RB_INSERT(prefix_index, >adj_rib_out, p) != NULL)
fatalx("%s: RB index invariant violated", __func__);
}
+
+   if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
+   fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
+   if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) {
+   if (prefix_nhflags(p) == state->nhflags &&
+   prefix_nexthop(p) == state->nexthop &&
+   communities_equal(>communities,
+   prefix_communities(p)) &&
+   path_compare(>aspath, prefix_aspath(p)) ==
+   0) {
+   /* nothing changed */
+   p->validation_state = vstate;
+   p->lastchange = getmonotime();
+   p->flags &= ~PREFIX_FLAG_STALE;
+   return;
+   }
+
+   /* if pending update unhook it before it is unlinked */
+   if (p->flags & PREFIX_FLAG_UPDATE) {
+   RB_REMOVE(prefix_tree, >updates[prefix->aid], p);
+   peer->up_nlricnt--;
+   }
+
+   /* unlink prefix so it can be relinked below */
+   prefix_unlink(p);
+   peer->prefix_out_cnt--;
+   }
+   if (p->flags & PREFIX_FLAG_WITHDRAW) {
+   RB_REMOVE(prefix_tree, >withdraws[prefix->aid], p);
+   peer->up_wcnt--;
+   }
+
+   /* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */
+   p->flags &= ~PREFIX_FLAG_MASK;
 
if ((asp = path_lookup(>aspath)) == NULL) {
/* Path not available, create and link a new one. */



PATCH: speed up pkg_add

2022-03-15 Thread Marc Espie
I've changed pkg_add for updates and tied files a while back, which resulted
in creating a lot of temp files. The logic was complicated, and I didn't
want to make it more complex at the time until most of the issues were fixed.

Then we got tags, so that most packing-lists are innocuous these days.

Now is that time, the following patch may speed up some updates and additions
significantly, in case the packing-lists don't contain "random execs".

What this does:
- in case we extract a new file that's not on the filesystem, we can extract
in-place instead of going through a temporary item

- in case we are matching files with identical files during an update, and
the filename doesn't change, we acknowledge we don't need to do anything.

Caveat: this may bite. It is advised to run pkg_check afterwards and
report file system discrepancies.


Index: OpenBSD/Add.pm
===
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/Add.pm,v
retrieving revision 1.187
diff -u -p -r1.187 Add.pm
--- OpenBSD/Add.pm  9 Mar 2022 12:27:51 -   1.187
+++ OpenBSD/Add.pm  15 Mar 2022 10:08:07 -
@@ -466,6 +466,7 @@ sub find_safe_dir
my $fullname = $self->fullname;
my $filename = $state->{destdir}.$fullname;
my $d = dirname($filename);
+   my $orig = $d;
 
# we go back up until we find an existing directory.
# hopefully this will be on the same file system.
@@ -483,6 +484,12 @@ sub find_safe_dir
if (!-e _ && !$state->{not}) {
$state->make_path($d, $fullname);
}
+   if ($state->{current_set}{simple_update} && 
+   $d eq $orig && 
+   !-e $filename) {
+   $self->{avoid_temp} = $filename;
+   }
+
return $d;
 }
 
@@ -503,6 +510,18 @@ sub create_temp
return ($fh, $tempname);
 }
 
+sub may_create_temp
+{
+   my ($self, $d, $state) = @_;
+   if ($self->{avoid_temp}) {
+   if (open(my $fh, '>', $self->{avoid_temp})) {
+   return ($fh, $self->{avoid_temp});
+   }
+   }
+   delete $self->{avoid_temp};
+   return $self->create_temp($d, $state);
+}
+
 sub tie
 {
my ($self, $state) = @_;
@@ -513,14 +532,22 @@ sub tie
$self->SUPER::extract($state);
 
my $d = $self->find_safe_dir($state);
+   my $src = $self->{tieto}->realname($state);
+   my $dest = $self->realname($state);
+   if ($state->{current_set}{simple_update} && $src eq $dest) {
+   $state->say("No name change on tied file #1", $src)
+   if $state->verbose >= 3;
+   $self->{tieto}{dont_delete} = 1;
+   $self->{avoid_temp} = 1;
+   return;
+   }
if ($state->{not}) {
$state->say("link #1 -> #2", 
$self->name, $d) if $state->verbose >= 3;
} else {
-   my ($fh, $tempname) = $self->create_temp($d, $state);
+   my ($fh, $tempname) = $self->may_create_temp($d, $state);
 
return if !defined $tempname;
-   my $src = $self->{tieto}->realname($state);
unlink($tempname);
$state->say("link #1 -> #2", $src, $tempname)
if $state->verbose >= 3;
@@ -528,6 +555,7 @@ sub tie
}
 }
 
+
 sub extract
 {
my ($self, $state, $file) = @_;
@@ -540,13 +568,13 @@ sub extract
$self->name, $d) if $state->verbose >= 3;
$state->{archive}->skip;
} else {
-   my ($fh, $tempname) = $self->create_temp($d, $state);
-   if (!defined $tempname) {
+   my ($fh, $filename) = $self->may_create_temp($d, $state);
+   if (!defined $filename) {
$state->{archive}->skip;
return;
}
 
-   $state->say("extract #1 -> #2", $self->name, $tempname) 
+   $state->say("extract #1 -> #2", $self->name, $filename) 
if $state->verbose >= 3;
 
 
@@ -555,7 +583,7 @@ sub extract
$self->stringize);
}
$file->extract_to_fh($fh);
-   $self->may_check_digest($tempname, $state);
+   $self->may_check_digest($filename, $state);
}
 }
 
@@ -576,17 +604,21 @@ sub install
} elsif (defined $self->{symlink}) {
symlink($self->{symlink}, $destdir.$fullname);
} else {
-   if (!defined $self->{tempname}) {
-   return if $state->allow_nonroot($fullname);
-   $state->fatal("No tempname for #1", $fullname);
+   if (defined $self->{avoid_temp}) {
+   delete $self->{avoid_temp};
+   } else {
+   if (!defined $self->{tempname}) {
+   return if $state->allow_nonroot($fullname);
+   

Re: Add refcnt_read()

2022-03-15 Thread Vitaliy Makkoveev
On Tue, Mar 15, 2022 at 09:11:30AM +, Visa Hankala wrote:
> This patch adds a function for getting a snapshot of a reference
> counter. This will let code like crcopy() access the value in an
> API-observing way.
> 
> OK?

ok mvs@

> 
> Index: share/man/man9/refcnt_init.9
> ===
> RCS file: src/share/man/man9/refcnt_init.9,v
> retrieving revision 1.1
> diff -u -p -r1.1 refcnt_init.9
> --- share/man/man9/refcnt_init.9  11 Sep 2015 19:13:22 -  1.1
> +++ share/man/man9/refcnt_init.9  15 Mar 2022 07:52:39 -
> @@ -23,6 +23,7 @@
>  .Nm refcnt_rele ,
>  .Nm refcnt_rele_wake ,
>  .Nm refcnt_finalize ,
> +.Nm refcnt_read ,
>  .Nm REFCNT_INITIALIZER
>  .Nd reference count API
>  .Sh SYNOPSIS
> @@ -37,6 +38,8 @@
>  .Fn "refcnt_rele_wake" "struct refcnt *r"
>  .Ft void
>  .Fn "refcnt_finalize" "struct refcnt *r" "const char *wmesg"
> +.Ft unsigned int
> +.Fn "refcnt_read" "struct refcnt *r"
>  .Fn "REFCNT_INITIALIZER"
>  .Sh DESCRIPTION
>  The refcnt API provides simple reference counters that can be used
> @@ -68,14 +71,21 @@ There may only be one caller to
>  per refcnt
>  .Fa r .
>  .Pp
> +.Fn refcnt_read
> +returns a snapshot of the counter value.
> +The value can become stale immediately if other CPUs are able to change
> +the counter in parallel.
> +The function does not enforce any memory access order.
> +.Pp
>  .Fn REFCNT_INITIALIZER
>  initialises a declaration of a refcnt to 1.
>  .Sh CONTEXT
>  .Fn refcnt_init ,
>  .Fn refcnt_take ,
>  .Fn refcnt_rele ,
> +.Fn refcnt_rele_wake ,
>  and
> -.Fn refcnt_rele_wake
> +.Fn refcnt_read
>  can be called during autoconf, from process context, or from interrupt
>  context.
>  .Pp
> @@ -85,3 +95,6 @@ can be called from process context.
>  .Fn refcnt_rele
>  returns a non-zero value if the last reference has been released,
>  otherwise 0.
> +.Pp
> +.Fn refcnt_read
> +returns a snapshot of the counter value.
> Index: sys/kern/kern_synch.c
> ===
> RCS file: src/sys/kern/kern_synch.c,v
> retrieving revision 1.183
> diff -u -p -r1.183 kern_synch.c
> --- sys/kern/kern_synch.c 10 Mar 2022 15:21:08 -  1.183
> +++ sys/kern/kern_synch.c 15 Mar 2022 07:52:39 -
> @@ -852,6 +852,12 @@ refcnt_finalize(struct refcnt *r, const 
>   }
>  }
>  
> +unsigned int
> +refcnt_read(struct refcnt *r)
> +{
> + return (atomic_load_int(>r_refs));
> +}
> +
>  void
>  cond_init(struct cond *c)
>  {
> Index: sys/sys/refcnt.h
> ===
> RCS file: src/sys/sys/refcnt.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 refcnt.h
> --- sys/sys/refcnt.h  10 Mar 2022 15:21:08 -  1.5
> +++ sys/sys/refcnt.h  15 Mar 2022 07:52:39 -
> @@ -37,6 +37,7 @@ voidrefcnt_take(struct refcnt *);
>  int  refcnt_rele(struct refcnt *);
>  void refcnt_rele_wake(struct refcnt *);
>  void refcnt_finalize(struct refcnt *, const char *);
> +unsigned int refcnt_read(struct refcnt *);
>  
>  #endif /* _KERNEL */
>  
> 



Remove data dependency barrier from atomic_load_*

2022-03-15 Thread Visa Hankala
This removes the data dependency consumer barrier from the atomic_load_*
functions. I think the intent was to keep these functions relaxed in
terms of CPU memory order.

This makes these functions more agreeable in code that assertions may
use, such as the suggested refcnt_read().

Removing the barrier should be safe at this point. The current callers
of atomic_load_*, that is cond_wait() and refcnt_finalize(), use the
loaded value for a control decision. These functions need to provide
a stronger ordering guarantee (memory acquire) for their callers than
what membar_datadep_consumer() gives.

(The data dependency barrier would be necessary in a setting like the
following where a memory load of non-constant data is dependent on
another load.

idx = atomic_load_int();
membar_datadep_consumer();
val = atomic_load_int([idx]);

Typically, even if the processor did reorder loads, the second load's
dependency on the value of the first load would prevent the load-load
reordering.

However, some DEC Alpha CPUs have their data caches divided into cache
banks to improve bandwidth. These cache banks are relatively
independent. The system maintains coherency, but bus contention can
delay propagation of cache updates. If the loads spanned different cache
banks, the second load could deliver data which is older than the
initial load's value. The data dependency barrier causes an interlock
with cache updating, ensuring causal ordering.)

OK?

Index: sys/sys/atomic.h
===
RCS file: src/sys/sys/atomic.h,v
retrieving revision 1.8
diff -u -p -r1.8 atomic.h
--- sys/sys/atomic.h11 Mar 2022 19:02:15 -  1.8
+++ sys/sys/atomic.h15 Mar 2022 07:52:39 -
@@ -201,26 +201,16 @@ atomic_sub_long_nv(volatile unsigned lon
  * atomic_load_* - read from memory
  */
 
-static inline void membar_datadep_consumer(void);
-
 static inline unsigned int
 atomic_load_int(volatile unsigned int *p)
 {
-   unsigned int v;
-
-   v = *p;
-   membar_datadep_consumer();
-   return v;
+   return *p;
 }
 
 static inline unsigned long
 atomic_load_long(volatile unsigned long *p)
 {
-   unsigned long v;
-
-   v = *p;
-   membar_datadep_consumer();
-   return v;
+   return *p;
 }
 
 /*



Add refcnt_read()

2022-03-15 Thread Visa Hankala
This patch adds a function for getting a snapshot of a reference
counter. This will let code like crcopy() access the value in an
API-observing way.

OK?

Index: share/man/man9/refcnt_init.9
===
RCS file: src/share/man/man9/refcnt_init.9,v
retrieving revision 1.1
diff -u -p -r1.1 refcnt_init.9
--- share/man/man9/refcnt_init.911 Sep 2015 19:13:22 -  1.1
+++ share/man/man9/refcnt_init.915 Mar 2022 07:52:39 -
@@ -23,6 +23,7 @@
 .Nm refcnt_rele ,
 .Nm refcnt_rele_wake ,
 .Nm refcnt_finalize ,
+.Nm refcnt_read ,
 .Nm REFCNT_INITIALIZER
 .Nd reference count API
 .Sh SYNOPSIS
@@ -37,6 +38,8 @@
 .Fn "refcnt_rele_wake" "struct refcnt *r"
 .Ft void
 .Fn "refcnt_finalize" "struct refcnt *r" "const char *wmesg"
+.Ft unsigned int
+.Fn "refcnt_read" "struct refcnt *r"
 .Fn "REFCNT_INITIALIZER"
 .Sh DESCRIPTION
 The refcnt API provides simple reference counters that can be used
@@ -68,14 +71,21 @@ There may only be one caller to
 per refcnt
 .Fa r .
 .Pp
+.Fn refcnt_read
+returns a snapshot of the counter value.
+The value can become stale immediately if other CPUs are able to change
+the counter in parallel.
+The function does not enforce any memory access order.
+.Pp
 .Fn REFCNT_INITIALIZER
 initialises a declaration of a refcnt to 1.
 .Sh CONTEXT
 .Fn refcnt_init ,
 .Fn refcnt_take ,
 .Fn refcnt_rele ,
+.Fn refcnt_rele_wake ,
 and
-.Fn refcnt_rele_wake
+.Fn refcnt_read
 can be called during autoconf, from process context, or from interrupt
 context.
 .Pp
@@ -85,3 +95,6 @@ can be called from process context.
 .Fn refcnt_rele
 returns a non-zero value if the last reference has been released,
 otherwise 0.
+.Pp
+.Fn refcnt_read
+returns a snapshot of the counter value.
Index: sys/kern/kern_synch.c
===
RCS file: src/sys/kern/kern_synch.c,v
retrieving revision 1.183
diff -u -p -r1.183 kern_synch.c
--- sys/kern/kern_synch.c   10 Mar 2022 15:21:08 -  1.183
+++ sys/kern/kern_synch.c   15 Mar 2022 07:52:39 -
@@ -852,6 +852,12 @@ refcnt_finalize(struct refcnt *r, const 
}
 }
 
+unsigned int
+refcnt_read(struct refcnt *r)
+{
+   return (atomic_load_int(>r_refs));
+}
+
 void
 cond_init(struct cond *c)
 {
Index: sys/sys/refcnt.h
===
RCS file: src/sys/sys/refcnt.h,v
retrieving revision 1.5
diff -u -p -r1.5 refcnt.h
--- sys/sys/refcnt.h10 Mar 2022 15:21:08 -  1.5
+++ sys/sys/refcnt.h15 Mar 2022 07:52:39 -
@@ -37,6 +37,7 @@ void  refcnt_take(struct refcnt *);
 intrefcnt_rele(struct refcnt *);
 void   refcnt_rele_wake(struct refcnt *);
 void   refcnt_finalize(struct refcnt *, const char *);
+unsigned int   refcnt_read(struct refcnt *);
 
 #endif /* _KERNEL */