btop++
Hi, I ported btop++ (a resource monitor program like top/htop) to macOS, FreeBSD, and now OpenBSD (pull request: https://github.com/aristocratos/btop/pull/607). I would appreciate if anyone could have a look and tell me what stupid things I did and how I can do it better (I don't have much OpenBSD experience). I have some questions that I can't seem to find an answer to: - is it possible to get the temperature per CPU thread? Right now I'm giving both threads on same physical core same temperature - same question about CPU usage, it seems only possible to get info on physical core granularity - Similar question about disk I/O, I can only find a way to get disk I/O per disk, so I'm giving all mountpoints the same disk I/O. - Is it possible to get disk I/O info per process? Linux and macOS can do that, in FreeBSD I also could not find an API for it. How could we go about including this in OpenBSD (ports or native package)? (Please CC me as I'm not subscribed to the list) Thx, Jos
Re: PATCH: remove "dead code" in make
On Sun, Sep 03, 2023 at 06:47:49PM +0200, Marc Espie wrote: > On Thu, Aug 31, 2023 at 08:59:53AM +0200, Marc Espie wrote: > > A long time ago, I tried to host our fork of make, in the hope it would get > > picked up by other systems. > > > > Accordingly, some features were added to mimic netbsd's extensions, hidden > > behind FEATURES macros. > > > > Turns out that, for better or for worse, FreeBSD decided to go with NetBSD's > > fork of bmake, so this is just dead weight that makes stuff more complex. > > > > Accordingly, config.h is no longer really needed, as it shrinks to two > > little defines. > > > > (the only part worth looking at is varmodifiers... this survived a full > > build > > of base and xenocara without any issue, along with quite a few ports) > > Slightly updated version. I hadn't bothered to actually remove config.h > and thus missed an include. > > Also: all remaining modifiers are now strictly either word_apply or apply, > none does both, so I killed the extra code and added an assert just in case. > > I could use some okays. Fairly safe. ok tb
Re: dt(4), hardclock(9): move interval, profile providers to dedicated callback
On 25/08/23(Fri) 21:00, Scott Cheloha wrote: > On Thu, Aug 24, 2023 at 07:21:29PM +0200, Martin Pieuchot wrote: > > On 23/08/23(Wed) 18:52, Scott Cheloha wrote: > > > This is the next patch in the clock interrupt reorganization series. > > > > Thanks for your diff. I'm sorry but it is really hard for me to help > > review this diff because there is still no man page for this API+subsystem. > > > > Can we start with that please? > > Sure, a first draft of a clockintr_establish.9 manpage is included > below. Lovely, I'll answer to that first in this email. Please commit it, so we can tweak it in tree. ok mpi@ > We also have a manpage in the tree, clockintr.9. It is a bit out of > date, but it covers the broad strokes of how the driver-facing portion > of the subsystem works. Currently reading it, should we get rid of `schedhz' in the manpage and its remaining in the kernel? Why isn't the statclock() always randomize? I see a couple of clocks/archs that do not use CL_RNDSTAT... Is there any technical reason apart from testing? I don't understand what you mean with "Until we understand scheduler lock contention better"? What does lock contention has to do with firing hardclock and statclock at the same moment? > Index: share/man/man9/clockintr_establish.9 > === > RCS file: share/man/man9/clockintr_establish.9 > diff -N share/man/man9/clockintr_establish.9 > --- /dev/null 1 Jan 1970 00:00:00 - > +++ share/man/man9/clockintr_establish.9 26 Aug 2023 01:44:37 - > @@ -0,0 +1,239 @@ > +.\" $OpenBSD$ > +.\" > +.\" Copyright (c) 2020-2023 Scott Cheloha > +.\" > +.\" Permission to use, copy, modify, and distribute this software for any > +.\" purpose with or without fee is hereby granted, provided that the above > +.\" copyright notice and this permission notice appear in all copies. > +.\" > +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > +.\" > +.Dd $Mdocdate$ > +.Dt CLOCKINTR_ESTABLISH 9 > +.Os > +.Sh NAME > +.Nm clockintr_advance , > +.Nm clockintr_cancel , > +.Nm clockintr_disestablish , > +.Nm clockintr_establish , > +.Nm clockintr_expiration , > +.Nm clockintr_nsecuptime , > +.Nm clockintr_schedule , > +.Nm clockintr_stagger > +.Nd schedule a function for execution from clock interrupt context > +.Sh SYNOPSIS > +.In sys/clockintr.h > +.Ft uint64_t > +.Fo clockintr_advance > +.Fa "struct clockintr *cl" > +.Fa "uint64_t interval" Can we do s/interval/nsecs/? > +.Fc > +.Ft void > +.Fo clockintr_cancel > +.Fa "struct clockintr *cl" > +.Fc > +.Ft void > +.Fo clockintr_disestablish > +.Fa "struct clockintr *cl" > +.Fc > +.Ft struct clockintr * > +.Fo clockintr_establish > +.Fa "struct clockintr_queue *queue" > +.Fa "void (*func)(struct clockintr *, void *)" > +.Fc > +.Ft uint64_t > +.Fo clockintr_expiration > +.Fa "const struct clockintr *cl" > +.Fc > +.Ft uint64_t > +.Fo clockintr_nsecuptime > +.Fa "const struct clockintr *cl" > +.Fc > +.Ft void > +.Fo clockintr_schedule > +.Fa "struct clockintr *cl" > +.Fa "uint64_t abs" > +.Fc > +.Ft void > +.Fo clockintr_stagger > +.Fa "struct clockintr *cl" > +.Fa "uint64_t interval" > +.Fa "u_int numerator" > +.Fa "u_int denominator" > +.Fc > +.Sh DESCRIPTION > +The clock interrupt subsystem schedules functions for asynchronous execution > +in the future from a hardware interrupt context. This is the same description as timeout_set(9) apart from "hardware interrupt context". Why should I use this one and not the other one? I'm confused. I dislike choices. > +.Pp > +The > +.Fn clockintr_establish > +function allocates a new clock interrupt object > +.Po > +a > +.Dq clockintr > +.Pc > +and binds it to the given clock interrupt > +.Fa queue . > +When the clockintr is executed, > +the callback function > +.Fa func > +will be called from a hardware interrupt context on the CPU in control of the > +.Fa queue . It seems clockintr queues are per-CPU, can we abstract this data structure from the caller and pass CPUs instead? I don't see the point of manipulating such queues outside of kern/kern_clockintr.c > +The new clockintr is not initially scheduled for execution and has an > +expiration time of zero. Speaking of expiration time confuse me. I don't see this as necessary, at least for the moment. When writing API I'd strongly suggest to not add functions that are not used. This applies to clockintr_expiration() below. > +.Pp > +The > +.Fn clockintr_schedule > +function schedules the given clockintr
Re: btop++
On 2023/09/04 11:13, Jos Dehaes wrote: > How could we go about including this in OpenBSD (ports or native package)? it would need to be added as a port, then packages will be built. ports cannot be built with gcc 11 (gcc 8 and 11 can't be installed together and other ports require gcc 8). so due to the hard dep on C++20 it will need some very new stuff in -current ports to be able to build with llvm 16 (this is so new that packages aren't available yet). attached tgz is a start at a port, but i might have got the llvm 16 bits wrong. btop.tgz Description: application/tar-gz
Re: btop++
On Mon, Sep 04, 2023 at 11:13:01AM +0200, Jos Dehaes wrote: > Hi, > > I ported btop++ (a resource monitor program like top/htop) to macOS, > FreeBSD, and now OpenBSD (pull request: > https://github.com/aristocratos/btop/pull/607). > > I would appreciate if anyone could have a look and tell me what stupid > things I did and how I can do it better (I don't have much OpenBSD > experience). > > I have some questions that I can't seem to find an answer to: > >- is it possible to get the temperature per CPU thread? Right now I'm >giving both threads on same physical core same temperature There is no physical temp sensor per thread. Only core temparatures make sense. So what you do is correct. >- same question about CPU usage, it seems only possible to get info on >physical core granularity In OpenBSD SMT cores show up as independent CPUs. So a 2 core 4 thread cpu has cpu0 - cpu3. >- Similar question about disk I/O, I can only find a way to get disk I/O >per disk, so I'm giving all mountpoints the same disk I/O. >From my knowledge there is no per mount stats. In general they do not really matter. >- Is it possible to get disk I/O info per process? Linux and macOS can >do that, in FreeBSD I also could not find an API for it. You can look at getrusage ru_inblock/ru_oublock and p_uru_inblock / p_uru_oublock from struct kinfo_proc. Not perfect but there is nothing else. > How could we go about including this in OpenBSD (ports or native package)? > > (Please CC me as I'm not subscribed to the list) > > Thx, > Jos -- :wq Claudio
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
On 2023-09-03 21:18, Dave Voutila wrote: Mischa writes: Nice!! Thanx Dave! Running go brrr as we speak. Testing with someone who is running Debian. Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time unless I hear of any issues. There are a couple of permanent VMs running on this host, 1 ToR node, OpenBSD VM and a Debian VM. While they were running I started my stress script. The first round I started 40 VMs with just bsd.rd, 2G memory All good, then I started 40 VMs with a base disk and 2G memory. After 20 VMs started I got the following messages on the console: [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: not MAP_STACK [umd159360/355276 sp=783369$96750 inside 7256d538c000-725645b8bFff: not MAP_STACK [umd172263/319211 sp=70fb86794b60 inside 75247a4d2000-75247acdifff: not MAP_STACK [umd142824/38950 sp=7db1ed2a64d0 inside 756c57d18000-756c58517fff: not MAP_STACK [umd19808/286658 sp=7dbied2a64d0 inside 70f685f41000-70f6867dofff: not MAP_STACK [umd193279/488634 sp=72652c3e3da0 inside 7845f168d000-7845f1e8cfff: not MAP_STACK [umd155924/286116 sp=7eac5a1ff060 inside 7b88bcb79000-7b88b4378fff: not MAP_STACK Not sure if this is related to starting of the VMs or something else, the ToR node was consuming 100%+ CPU at the time. :) Mischa
Re: dt(4), hardclock(9): move interval, profile providers to dedicated callback
On 25/08/23(Fri) 21:00, Scott Cheloha wrote: > On Thu, Aug 24, 2023 at 07:21:29PM +0200, Martin Pieuchot wrote: > > [...] > > The only behavior that needs to be preserved is the output of dumping > > stacks. That means DT_FA_PROFILE and DT_FA_STATIC certainly needs to > > be adapted with this change. You can figure that out by looking at the > > output of /usr/src/share/btrace/kprofile.bt without and with this diff. > > > > Please generate a FlameGraph to make sure they're still the same. > > dt_prov_profile_intr() runs at the same stack depth as hardclock(), so > indeed they are still the same. Lovely. > > Apart from that I'd prefer if we could skip the mechanical change and > > go straight to what dt(4) needs. Otherwise we will have to re-design > > everything. > > I think a mechanical "move the code from point A to point B" patch is > useful. It makes the changes easier to follow when tracing the > revision history in the future. > > If you insist on skipping it, though, I think I can live without it. I do insist. It is really hard for me to follow and work with you because you're too verbose for my capacity. If you want to work with me, please do smaller steps and do not mix so much in big diffs. I have plenty of possible comments but can deal with huge chunks. > > The current code assumes the periodic entry points are external to dt(4). > > This diff moves them in the middle of dt(4) but keeps the existing flow > > which makes the code very convoluted. > > > > A starting point to understand the added complexity it so see that the > > DT_ENTER() macro are no longer needed if we move the entry points inside > > dt(4). > > I did see that. It seems really easy to remove the macros in a > subsequent patch, though. > > Again, if you want to do it all in one patch that's OK. Yes please. > > The first periodic timeout is dt_prov_interval_enter(). It could be > > implemented as a per-PCB timeout_add_nsec(9). The drawback of this > > approach is that it uses too much code in the kernel which is a problem > > when instrumenting the kernel itself. Every subsystem used by dt(4) is > > impossible to instrument with btrace(8). > > I think you can avoid this instrumentation problem by using clockintr, > where the callback functions are run from the hardware interrupt > context, just like hardclock(). Fair enough. > > The second periodic timeout it dt_prov_profile_enter(). It is similar > > to the previous one and has to run on every CPU. > > > > Both are currently bound to tick, but we want per-PCB time resolution. > > We can get rid of `dp_nticks' and `dp_maxtick' if we control when the > > timeouts fires. > > My current thought is that each PCB could have its own "dp_period" or > "dp_interval", a count of nanoseconds. Indeed. We can have `dp_nsecs' and use that to determine if clockintr_advance() needs to be called in dt_ioctl_record_start(). > > > - Each dt_pcb gets a provider-specific "dp_clockintr" pointer. If the > > > PCB's implementing provider needs a clock interrupt to do its work, it > > > stores the handle in dp_clockintr. The handle, if any, is established > > > during dtpv_alloc() and disestablished during dtpv_dealloc(). > > > > Sorry, but as I said I don't understand what is a clockintr. How does it > > fit in the kernel and how is it supposed to be used? > > The goal of clockintr is to provide a machine-independent API for > scheduling clock interrupts. You can use it to implement something > like hardclock() or statclock(). We are already using it to implement > these functions, among others. After reading all the code and the previous manuals, I understand it as a low-level per-CPU timeout API with nanosecond precision. Is that it? > > Why have it per PCB and not per provider or for the whole driver instead? > > Per-PCB implies that if I run 3 different profiling on a 32 CPU machines > > I now have 96 different clockintr. Is it what we want? > > Yes, I think that sounds fine. If we run into scaling problems we can > always just change the underlying data structure to an RB tree or a > minheap. Fine. > > If so, why not simply use timeout(9)? What's the difference? > > Some code needs to run from a hardware interrupt context. It's going > to be easier to profile more of the kernel if we're collecting stack > traces during a clock interrupt. > > Timeouts run at IPL_SOFTCLOCK. You can profile process context > code... that's it. Timeouts also only run on a single CPU. Per-CPU > timeout wheels are at least a year away if not longer. It's not clear to me which code needs what. I'd love to see that explained in the manual. > > > One alternative is to start running the clock interrupts when they > > > are allocated in dtpv_alloc() and stop them when they are freed in > > > dtpv_dealloc(). This is wasteful, though: the PCBs are not recording > > > yet, so the interrupts won't perform any useful work until the > > > controlling proc
pax(1) and tar(1): fix misleading -DSMALL
Two code sets are currently guarded with #ifdef SMALL in pax(1) and tar(1): reading 'pax' format extended headers, and identifying various compressed formats for user-friendliness. As noted by Caspar, the SMALL path isn't currently used on the install media. I've been confused by this twice already... Here's a proposal: 1. always compile in read support for the 'pax' format extended headers. The ustar format is limited and being able to restore archives using the pax format in any situation would be nice. Especially if we switch to writing out pax format archives by default one day. We're definitely not there yet. 2. actually use -DSMALL to save a bit of storage on the install media. The behavior is still sane, tar(1) warns that it doesn't recognize a compressed archive, seeks through it trying to look a tar header, and eventually gives up. Here's the tiny size change on amd64: shannon /usr/src/distrib/special/pax$ size tar.o options.o pax obj/tar.o obj/options.o obj/pax textdatabss dec hex 68210 40 68611acdtar.o 7195108432 83112077options.o 390495 19024 85392 494911 78d3f pax 68210 40 68611acdobj/tar.o 6878108432 79941f3aobj/options.o 390175 19024 85392 494591 78bff obj/pax I don't expect any regression on the ramdisks but a make release is running just in case. ok? Index: bin/pax/tar.c === RCS file: /home/cvs/src/bin/pax/tar.c,v retrieving revision 1.72 diff -u -p -r1.72 tar.c --- bin/pax/tar.c 19 Aug 2023 04:21:05 - 1.72 +++ bin/pax/tar.c 4 Sep 2023 12:19:39 - @@ -59,9 +59,7 @@ static u_long tar_chksm(char *, int); static char *name_split(char *, int); static int ul_oct(u_long, char *, int, int); static int ull_oct(unsigned long long, char *, int, int); -#ifndef SMALL static int rd_xheader(ARCHD *arcn, int, off_t); -#endif static uid_t uid_nobody; static uid_t uid_warn; @@ -721,14 +719,11 @@ ustar_rd(ARCHD *arcn, char *buf) if (ustar_id(buf, BLKMULT) < 0) return(-1); -#ifndef SMALL reset: -#endif memset(arcn, 0, sizeof(*arcn)); arcn->org_name = arcn->name; arcn->sb.st_nlink = 1; -#ifndef SMALL /* Process Extended headers. */ if (hd->typeflag == XHDRTYPE || hd->typeflag == GHDRTYPE) { if (rd_xheader(arcn, hd->typeflag == GHDRTYPE, @@ -745,7 +740,6 @@ reset: if (hd->typeflag == XHDRTYPE || hd->typeflag == GHDRTYPE) goto reset; } -#endif if (!arcn->nlen) { /* @@ -1190,8 +1184,6 @@ expandname(char *buf, size_t len, char * return(nlen); } -#ifndef SMALL - /* shortest possible extended record: "5 a=\n" */ #define MINXHDRSZ 5 @@ -1331,4 +1323,3 @@ rd_xheader(ARCHD *arcn, int global, off_ return (-1); return (ret); } -#endif Index: distrib/special/pax/Makefile === RCS file: /home/cvs/src/distrib/special/pax/Makefile,v retrieving revision 1.2 diff -u -p -r1.2 Makefile --- distrib/special/pax/Makefile13 Sep 2018 16:34:33 - 1.2 +++ distrib/special/pax/Makefile3 Sep 2023 10:38:17 - @@ -1,7 +1,7 @@ # $OpenBSD: Makefile,v 1.2 2018/09/13 16:34:33 sthen Exp $ .PATH: ${.CURDIR}/../../../bin/pax -CFLAGS+=-DNOCPIO -I${.CURDIR}/../../../bin/pax +CFLAGS+=-DNOCPIO -DSMALL -I${.CURDIR}/../../../bin/pax PROG= pax SRCS= ar_io.c ar_subs.c buf_subs.c file_subs.c ftree.c\ -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
tcp sync cache signed use counter
Hi, Variable scs_use is basically counting packet insertions to syn cache, so I would prefer type long to exclude overflow on fast machines. With the current limits int should be enough, but long does not hurt. It can be negative as it starts at a positive limit and counts backwards. After all entries in the current syn cache have been timed out, it is reset to positive limit. If timeout takes a while, it may get well below zero. To prevent netstat output like this 18446744073709531826 uses of current SYN cache left make tcps_sc_uses_left signed and print it as long long integer. ok? bluhm Index: sys/netinet/tcp_var.h === RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v retrieving revision 1.170 diff -u -p -r1.170 tcp_var.h --- sys/netinet/tcp_var.h 28 Aug 2023 14:50:02 - 1.170 +++ sys/netinet/tcp_var.h 4 Sep 2023 13:02:40 - @@ -288,9 +288,9 @@ struct syn_cache_head { struct syn_cache_set { struct syn_cache_head *scs_buckethead; + longscs_use; int scs_size; int scs_count; - int scs_use; u_int32_t scs_random[5]; }; @@ -430,7 +430,7 @@ struct tcpstat { u_int64_t tcps_sc_entry_limit; /* limit of syn cache entries */ u_int64_t tcps_sc_bucket_maxlen;/* maximum # of entries in any bucket */ u_int64_t tcps_sc_bucket_limit; /* limit of syn cache bucket list */ - u_int64_t tcps_sc_uses_left;/* use counter of current syn cache */ + int64_t tcps_sc_uses_left; /* use counter of current syn cache */ u_int64_t tcps_conndrained; /* # of connections drained */ Index: usr.bin/netstat/inet.c === RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v retrieving revision 1.178 diff -u -p -r1.178 inet.c --- usr.bin/netstat/inet.c 7 Jul 2023 09:15:13 - 1.178 +++ usr.bin/netstat/inet.c 4 Sep 2023 12:51:01 - @@ -498,7 +498,7 @@ tcp_stats(char *name) "\t%llu entr%s in current SYN cache, limit is %llu\n"); p2b(tcps_sc_bucket_maxlen, tcps_sc_bucket_limit, "\t%llu longest bucket length in current SYN cache, limit is %llu\n"); - p(tcps_sc_uses_left, "\t%llu use%s of current SYN cache left\n"); + p(tcps_sc_uses_left, "\t%lld use%s of current SYN cache left\n"); p(tcps_sack_recovery_episode, "\t%llu SACK recovery episode%s\n"); p(tcps_sack_rexmits,
unifdef HAS_INLINES in make(1)
inline is part of gnu89 and c99 Index: defines.h === RCS file: /cvs/src/usr.bin/make/defines.h,v retrieving revision 1.15 diff -u -p -r1.15 defines.h --- defines.h 14 Oct 2015 13:50:22 - 1.15 +++ defines.h 4 Sep 2023 13:49:16 - @@ -61,16 +61,8 @@ typedef struct Suff_ Suff; #ifdef __GNUC__ # define UNUSED__attribute__((__unused__)) -# define HAS_INLINES -# define INLINE __inline__ #else # define UNUSED -#endif - -#ifdef HAS_INLINES -# ifndef INLINE -# define INLINE inline -# endif #endif /* Index: lst.h === RCS file: /cvs/src/usr.bin/make/lst.h,v retrieving revision 1.33 diff -u -p -r1.33 lst.h --- lst.h 4 Mar 2021 09:45:31 - 1.33 +++ lst.h 4 Sep 2023 13:49:52 - @@ -159,25 +159,16 @@ extern void * Lst_DeQueue(Lst); #define Lst_Adv(ln)((ln)->nextPtr) #define Lst_Rev(ln)((ln)->prevPtr) - -/* Inlines are preferable to macros here because of the type checking. */ -#ifdef HAS_INLINES -static INLINE LstNode +static inline LstNode Lst_FindConst(Lst l, FindProcConst cProc, const void *d) { return Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d); } -static INLINE LstNode +static inline LstNode Lst_FindFromConst(LstNode ln, FindProcConst cProc, const void *d) { return Lst_FindFrom(ln, (FindProc)cProc, (void *)d); } -#else -#define Lst_FindConst(l, cProc, d) \ - Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d) -#define Lst_FindFromConst(ln, cProc, d) \ - Lst_FindFrom(ln, (FindProc)cProc, (void *)d) -#endif #endif /* _LST_H_ */ Index: lst.lib/lst.h === RCS file: /cvs/src/usr.bin/make/lst.lib/lst.h,v retrieving revision 1.2 diff -u -p -r1.2 lst.h --- lst.lib/lst.h 14 Oct 2015 13:52:11 - 1.2 +++ lst.lib/lst.h 4 Sep 2023 13:50:30 - @@ -154,25 +154,16 @@ extern void * Lst_DeQueue(Lst); #define Lst_Adv(ln)((ln)->nextPtr) #define Lst_Rev(ln)((ln)->prevPtr) - -/* Inlines are preferable to macros here because of the type checking. */ -#ifdef HAS_INLINES -static INLINE LstNode +static inline LstNode Lst_FindConst(Lst l, FindProcConst cProc, const void *d) { return Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d); } -static INLINE LstNode +static inline LstNode Lst_FindFromConst(LstNode ln, FindProcConst cProc, const void *d) { return Lst_FindFrom(ln, (FindProc)cProc, (void *)d); } -#else -#define Lst_FindConst(l, cProc, d) \ - Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d) -#define Lst_FindFromConst(ln, cProc, d) \ - Lst_FindFrom(ln, (FindProc)cProc, (void *)d) -#endif #endif /* _LST_H_ */
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
On Mon, Sep 04, 2023 at 02:30:23PM +0200, Mischa wrote: > On 2023-09-03 21:18, Dave Voutila wrote: > > Mischa writes: > > > > > Nice!! Thanx Dave! > > > > > > Running go brrr as we speak. > > > Testing with someone who is running Debian. > > > > Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time > > unless I hear of any issues. > > There are a couple of permanent VMs running on this host, 1 ToR node, > OpenBSD VM and a Debian VM. > While they were running I started my stress script. > The first round I started 40 VMs with just bsd.rd, 2G memory > All good, then I started 40 VMs with a base disk and 2G memory. > After 20 VMs started I got the following messages on the console: > > [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: not > MAP_STACK > [umd159360/355276 sp=783369$96750 inside 7256d538c000-725645b8bFff: not > MAP_STACK > [umd172263/319211 sp=70fb86794b60 inside 75247a4d2000-75247acdifff: not > MAP_STACK > [umd142824/38950 sp=7db1ed2a64d0 inside 756c57d18000-756c58517fff: not > MAP_STACK > [umd19808/286658 sp=7dbied2a64d0 inside 70f685f41000-70f6867dofff: not > MAP_STACK > [umd193279/488634 sp=72652c3e3da0 inside 7845f168d000-7845f1e8cfff: not > MAP_STACK > [umd155924/286116 sp=7eac5a1ff060 inside 7b88bcb79000-7b88b4378fff: not > MAP_STACK > > Not sure if this is related to starting of the VMs or something else, the > ToR node was consuming 100%+ CPU at the time. :) > > Mischa I have not seen this; can you try without the ToR node some time and see if this still happens?
Re: pax(1) and tar(1): fix misleading -DSMALL
On Mon, 04 Sep 2023 13:54:18 +0100, Jeremie Courreges-Anglas wrote: > Two code sets are currently guarded with #ifdef SMALL in pax(1) and > tar(1): reading 'pax' format extended headers, and identifying various > compressed formats for user-friendliness. As noted by Caspar, the SMALL > path isn't currently used on the install media. I've been confused by > this twice already... > > Here's a proposal: > > 1. always compile in read support for the 'pax' format extended headers. > The ustar format is limited and being able to restore archives using > the pax format in any situation would be nice. Especially if we > switch to writing out pax format archives by default one day. > We're definitely not there yet. Agreed. > 2. actually use -DSMALL to save a bit of storage on the install media. > The behavior is still sane, tar(1) warns that it doesn't recognize > a compressed archive, seeks through it trying to look a tar header, > and eventually gives up. Here's the tiny size change on amd64: > shannon /usr/src/distrib/special/pax$ size tar.o options.o pax obj/tar.o > obj/options.o obj/pax > textdatabss dec hex > 68210 40 68611acdtar.o > 7195108432 83112077options.o > 390495 19024 85392 494911 78d3f pax > 68210 40 68611acdobj/tar.o > 6878108432 79941f3aobj/options.o > 390175 19024 85392 494591 78bff obj/pax > > I don't expect any regression on the ramdisks but a make release is > running just in case. Sure, nothing relies on that anyway. - todd
Re: btop++
On Mon, 4 Sept 2023 at 14:14, Stuart Henderson wrote: > On 2023/09/04 11:13, Jos Dehaes wrote: > > How could we go about including this in OpenBSD (ports or native > package)? > > it would need to be added as a port, then packages will be built. > > ports cannot be built with gcc 11 (gcc 8 and 11 can't be installed > together and other ports require gcc 8). > > so due to the hard dep on C++20 it will need some very new stuff in > -current ports to be able to build with llvm 16 (this is so new that > packages aren't available yet). > > attached tgz is a start at a port, but i might have got the llvm 16 > bits wrong. > Thanks, I tried to build it, but apparently it builds using clang 13 which fails. Any tips on how to solve this? Do I first manually need to build clang 16?
Re: pax(1) and tar(1): fix misleading -DSMALL
On Mon, Sep 04, 2023 at 01:54:18PM +0100, Jeremie Courreges-Anglas wrote: > > Two code sets are currently guarded with #ifdef SMALL in pax(1) and > tar(1): reading 'pax' format extended headers, and identifying various > compressed formats for user-friendliness. As noted by Caspar, the SMALL > path isn't currently used on the install media. I've been confused by > this twice already... > > Here's a proposal: > > 1. always compile in read support for the 'pax' format extended headers. > The ustar format is limited and being able to restore archives using > the pax format in any situation would be nice. Especially if we > switch to writing out pax format archives by default one day. > We're definitely not there yet. > > 2. actually use -DSMALL to save a bit of storage on the install media. > The behavior is still sane, tar(1) warns that it doesn't recognize > a compressed archive, seeks through it trying to look a tar header, > and eventually gives up. Here's the tiny size change on amd64: > shannon /usr/src/distrib/special/pax$ size tar.o options.o pax obj/tar.o > obj/options.o obj/pax > textdatabss dec hex > 68210 40 68611acdtar.o > 7195108432 83112077options.o > 390495 19024 85392 494911 78d3f pax > 68210 40 68611acdobj/tar.o > 6878108432 79941f3aobj/options.o > 390175 19024 85392 494591 78bff obj/pax > > I don't expect any regression on the ramdisks but a make release is > running just in case. > > ok? Thanks a lot. OK caspar@ provided that Todd and Philip don't object. Caspar > > > Index: bin/pax/tar.c > === > RCS file: /home/cvs/src/bin/pax/tar.c,v > retrieving revision 1.72 > diff -u -p -r1.72 tar.c > --- bin/pax/tar.c 19 Aug 2023 04:21:05 - 1.72 > +++ bin/pax/tar.c 4 Sep 2023 12:19:39 - > @@ -59,9 +59,7 @@ static u_long tar_chksm(char *, int); > static char *name_split(char *, int); > static int ul_oct(u_long, char *, int, int); > static int ull_oct(unsigned long long, char *, int, int); > -#ifndef SMALL > static int rd_xheader(ARCHD *arcn, int, off_t); > -#endif > > static uid_t uid_nobody; > static uid_t uid_warn; > @@ -721,14 +719,11 @@ ustar_rd(ARCHD *arcn, char *buf) > if (ustar_id(buf, BLKMULT) < 0) > return(-1); > > -#ifndef SMALL > reset: > -#endif > memset(arcn, 0, sizeof(*arcn)); > arcn->org_name = arcn->name; > arcn->sb.st_nlink = 1; > > -#ifndef SMALL > /* Process Extended headers. */ > if (hd->typeflag == XHDRTYPE || hd->typeflag == GHDRTYPE) { > if (rd_xheader(arcn, hd->typeflag == GHDRTYPE, > @@ -745,7 +740,6 @@ reset: > if (hd->typeflag == XHDRTYPE || hd->typeflag == GHDRTYPE) > goto reset; > } > -#endif > > if (!arcn->nlen) { > /* > @@ -1190,8 +1184,6 @@ expandname(char *buf, size_t len, char * > return(nlen); > } > > -#ifndef SMALL > - > /* shortest possible extended record: "5 a=\n" */ > #define MINXHDRSZ5 > > @@ -1331,4 +1323,3 @@ rd_xheader(ARCHD *arcn, int global, off_ > return (-1); > return (ret); > } > -#endif > Index: distrib/special/pax/Makefile > === > RCS file: /home/cvs/src/distrib/special/pax/Makefile,v > retrieving revision 1.2 > diff -u -p -r1.2 Makefile > --- distrib/special/pax/Makefile 13 Sep 2018 16:34:33 - 1.2 > +++ distrib/special/pax/Makefile 3 Sep 2023 10:38:17 - > @@ -1,7 +1,7 @@ > #$OpenBSD: Makefile,v 1.2 2018/09/13 16:34:33 sthen Exp $ > > .PATH: ${.CURDIR}/../../../bin/pax > -CFLAGS+=-DNOCPIO -I${.CURDIR}/../../../bin/pax > +CFLAGS+=-DNOCPIO -DSMALL -I${.CURDIR}/../../../bin/pax > > PROG= pax > SRCS=ar_io.c ar_subs.c buf_subs.c file_subs.c ftree.c\ > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
On 2023-09-04 16:23, Mike Larkin wrote: On Mon, Sep 04, 2023 at 02:30:23PM +0200, Mischa wrote: On 2023-09-03 21:18, Dave Voutila wrote: > Mischa writes: > > > Nice!! Thanx Dave! > > > > Running go brrr as we speak. > > Testing with someone who is running Debian. > > Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time > unless I hear of any issues. There are a couple of permanent VMs running on this host, 1 ToR node, OpenBSD VM and a Debian VM. While they were running I started my stress script. The first round I started 40 VMs with just bsd.rd, 2G memory All good, then I started 40 VMs with a base disk and 2G memory. After 20 VMs started I got the following messages on the console: [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: not MAP_STACK [umd159360/355276 sp=783369$96750 inside 7256d538c000-725645b8bFff: not MAP_STACK [umd172263/319211 sp=70fb86794b60 inside 75247a4d2000-75247acdifff: not MAP_STACK [umd142824/38950 sp=7db1ed2a64d0 inside 756c57d18000-756c58517fff: not MAP_STACK [umd19808/286658 sp=7dbied2a64d0 inside 70f685f41000-70f6867dofff: not MAP_STACK [umd193279/488634 sp=72652c3e3da0 inside 7845f168d000-7845f1e8cfff: not MAP_STACK [umd155924/286116 sp=7eac5a1ff060 inside 7b88bcb79000-7b88b4378fff: not MAP_STACK Not sure if this is related to starting of the VMs or something else, the ToR node was consuming 100%+ CPU at the time. :) Mischa I have not seen this; can you try without the ToR node some time and see if this still happens? Testing again without any other VMs running. Things wrong when I run the following command and wait a little. for i in $(jot 10 10); do vmctl create -b /var/vmm/vm09.qcow2 /var/vmm/vm${i}.qcow2 && vmctl start -L -d /var/vmm/vm${i}.qcow2 -m 2G vm${i}; done Mischa
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
Mischa writes: > On 2023-09-04 16:23, Mike Larkin wrote: >> On Mon, Sep 04, 2023 at 02:30:23PM +0200, Mischa wrote: >>> On 2023-09-03 21:18, Dave Voutila wrote: >>> > Mischa writes: >>> > >>> > > Nice!! Thanx Dave! >>> > > >>> > > Running go brrr as we speak. >>> > > Testing with someone who is running Debian. >>> > >>> > Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time >>> > unless I hear of any issues. >>> There are a couple of permanent VMs running on this host, 1 ToR >>> node, >>> OpenBSD VM and a Debian VM. >>> While they were running I started my stress script. >>> The first round I started 40 VMs with just bsd.rd, 2G memory >>> All good, then I started 40 VMs with a base disk and 2G memory. >>> After 20 VMs started I got the following messages on the console: >>> [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: >>> not >>> MAP_STACK >>> [umd159360/355276 sp=783369$96750 inside 7256d538c000-725645b8bFff: >>> not >>> MAP_STACK >>> [umd172263/319211 sp=70fb86794b60 inside 75247a4d2000-75247acdifff: >>> not >>> MAP_STACK >>> [umd142824/38950 sp=7db1ed2a64d0 inside 756c57d18000-756c58517fff: not >>> MAP_STACK >>> [umd19808/286658 sp=7dbied2a64d0 inside 70f685f41000-70f6867dofff: not >>> MAP_STACK >>> [umd193279/488634 sp=72652c3e3da0 inside 7845f168d000-7845f1e8cfff: >>> not >>> MAP_STACK >>> [umd155924/286116 sp=7eac5a1ff060 inside 7b88bcb79000-7b88b4378fff: >>> not >>> MAP_STACK >>> Not sure if this is related to starting of the VMs or something >>> else, the >>> ToR node was consuming 100%+ CPU at the time. :) >>> Mischa >> I have not seen this; can you try without the ToR node some time and >> see if >> this still happens? > > Testing again without any other VMs running. > Things wrong when I run the following command and wait a little. > > for i in $(jot 10 10); do vmctl create -b /var/vmm/vm09.qcow2 > /var/vmm/vm${i}.qcow2 && vmctl start -L -d /var/vmm/vm${i}.qcow2 -m 2G > vm${i}; done Can you try adding a "sleep 2" or something in the loop? I can't think of a reason my changes would cause this. Do you see this on -current without the diff? -dv
Re: tcp sync cache signed use counter
> On 4 Sep 2023, at 16:19, Alexander Bluhm wrote: > > Hi, > > Variable scs_use is basically counting packet insertions to syn > cache, so I would prefer type long to exclude overflow on fast > machines. With the current limits int should be enough, but long > does not hurt. > > It can be negative as it starts at a positive limit and counts > backwards. After all entries in the current syn cache have been > timed out, it is reset to positive limit. If timeout takes a while, > it may get well below zero. > > To prevent netstat output like this >18446744073709531826 uses of current SYN cache left > make tcps_sc_uses_left signed and print it as long long integer. > > ok? > Does the negative value output makes sense? If not, could you prevent negative value output too? > bluhm > > Index: sys/netinet/tcp_var.h > === > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v > retrieving revision 1.170 > diff -u -p -r1.170 tcp_var.h > --- sys/netinet/tcp_var.h 28 Aug 2023 14:50:02 - 1.170 > +++ sys/netinet/tcp_var.h 4 Sep 2023 13:02:40 - > @@ -288,9 +288,9 @@ struct syn_cache_head { > > struct syn_cache_set { > struct syn_cache_head *scs_buckethead; > + longscs_use; > int scs_size; > int scs_count; > - int scs_use; > u_int32_t scs_random[5]; > }; > > @@ -430,7 +430,7 @@ structtcpstat { > u_int64_t tcps_sc_entry_limit; /* limit of syn cache entries */ > u_int64_t tcps_sc_bucket_maxlen;/* maximum # of entries in any bucket */ > u_int64_t tcps_sc_bucket_limit; /* limit of syn cache bucket list */ > - u_int64_t tcps_sc_uses_left;/* use counter of current syn cache */ > + int64_t tcps_sc_uses_left; /* use counter of current syn cache */ > > u_int64_t tcps_conndrained; /* # of connections drained */ > > Index: usr.bin/netstat/inet.c > === > RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v > retrieving revision 1.178 > diff -u -p -r1.178 inet.c > --- usr.bin/netstat/inet.c7 Jul 2023 09:15:13 - 1.178 > +++ usr.bin/netstat/inet.c4 Sep 2023 12:51:01 - > @@ -498,7 +498,7 @@ tcp_stats(char *name) > "\t%llu entr%s in current SYN cache, limit is %llu\n"); > p2b(tcps_sc_bucket_maxlen, tcps_sc_bucket_limit, > "\t%llu longest bucket length in current SYN cache, limit is > %llu\n"); > - p(tcps_sc_uses_left, "\t%llu use%s of current SYN cache left\n"); > + p(tcps_sc_uses_left, "\t%lld use%s of current SYN cache left\n"); > > p(tcps_sack_recovery_episode, "\t%llu SACK recovery episode%s\n"); > p(tcps_sack_rexmits, >
Re: tcp sync cache signed use counter
On Mon, Sep 04, 2023 at 07:22:03PM +0300, Vitaliy Makkoveev wrote: > > On 4 Sep 2023, at 16:19, Alexander Bluhm wrote: > > > > Hi, > > > > Variable scs_use is basically counting packet insertions to syn > > cache, so I would prefer type long to exclude overflow on fast > > machines. With the current limits int should be enough, but long > > does not hurt. > > > > It can be negative as it starts at a positive limit and counts > > backwards. After all entries in the current syn cache have been > > timed out, it is reset to positive limit. If timeout takes a while, > > it may get well below zero. > > > > To prevent netstat output like this > >18446744073709531826 uses of current SYN cache left > > make tcps_sc_uses_left signed and print it as long long integer. > > > > ok? > > > > Does the negative value output makes sense? If not, could you > prevent negative value output too? Goal is to reseed the hash algorithm from time to time. This cannot be done while there are still entries in the hash table. So we have two syn caches, an active and an inactive one. They switch roles when the active syn cache has been used for a while and the inactive one is empty. The scs_use counts the packets that have been inserted in the active one. We start at a limit that has been set by sysctl. We count down to 0, then use the inactive one. But it may be busy, so we wait until all entries have timed out. While that the active one is still used, and the counter gets negative. Negative numbers express how long the active syn cache has been used above the limit. Of course I could move the range to have positive numbers only. But then the state "used above limit" would not be obvious anymore. bluhm > > Index: sys/netinet/tcp_var.h > > === > > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v > > retrieving revision 1.170 > > diff -u -p -r1.170 tcp_var.h > > --- sys/netinet/tcp_var.h 28 Aug 2023 14:50:02 - 1.170 > > +++ sys/netinet/tcp_var.h 4 Sep 2023 13:02:40 - > > @@ -288,9 +288,9 @@ struct syn_cache_head { > > > > struct syn_cache_set { > > struct syn_cache_head *scs_buckethead; > > + longscs_use; > > int scs_size; > > int scs_count; > > - int scs_use; > > u_int32_t scs_random[5]; > > }; > > > > @@ -430,7 +430,7 @@ struct tcpstat { > > u_int64_t tcps_sc_entry_limit; /* limit of syn cache entries */ > > u_int64_t tcps_sc_bucket_maxlen;/* maximum # of entries in any bucket */ > > u_int64_t tcps_sc_bucket_limit; /* limit of syn cache bucket list */ > > - u_int64_t tcps_sc_uses_left;/* use counter of current syn cache */ > > + int64_t tcps_sc_uses_left; /* use counter of current syn cache */ > > > > u_int64_t tcps_conndrained; /* # of connections drained */ > > > > Index: usr.bin/netstat/inet.c > > === > > RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v > > retrieving revision 1.178 > > diff -u -p -r1.178 inet.c > > --- usr.bin/netstat/inet.c 7 Jul 2023 09:15:13 - 1.178 > > +++ usr.bin/netstat/inet.c 4 Sep 2023 12:51:01 - > > @@ -498,7 +498,7 @@ tcp_stats(char *name) > > "\t%llu entr%s in current SYN cache, limit is %llu\n"); > > p2b(tcps_sc_bucket_maxlen, tcps_sc_bucket_limit, > > "\t%llu longest bucket length in current SYN cache, limit is > > %llu\n"); > > - p(tcps_sc_uses_left, "\t%llu use%s of current SYN cache left\n"); > > + p(tcps_sc_uses_left, "\t%lld use%s of current SYN cache left\n"); > > > > p(tcps_sack_recovery_episode, "\t%llu SACK recovery episode%s\n"); > > p(tcps_sack_rexmits, > >
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
On 2023-09-04 17:57, Dave Voutila wrote: Mischa writes: On 2023-09-04 16:23, Mike Larkin wrote: On Mon, Sep 04, 2023 at 02:30:23PM +0200, Mischa wrote: On 2023-09-03 21:18, Dave Voutila wrote: > Mischa writes: > > > Nice!! Thanx Dave! > > > > Running go brrr as we speak. > > Testing with someone who is running Debian. > > Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time > unless I hear of any issues. There are a couple of permanent VMs running on this host, 1 ToR node, OpenBSD VM and a Debian VM. While they were running I started my stress script. The first round I started 40 VMs with just bsd.rd, 2G memory All good, then I started 40 VMs with a base disk and 2G memory. After 20 VMs started I got the following messages on the console: [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: not MAP_STACK [umd159360/355276 sp=783369$96750 inside 7256d538c000-725645b8bFff: not MAP_STACK [umd172263/319211 sp=70fb86794b60 inside 75247a4d2000-75247acdifff: not MAP_STACK [umd142824/38950 sp=7db1ed2a64d0 inside 756c57d18000-756c58517fff: not MAP_STACK [umd19808/286658 sp=7dbied2a64d0 inside 70f685f41000-70f6867dofff: not MAP_STACK [umd193279/488634 sp=72652c3e3da0 inside 7845f168d000-7845f1e8cfff: not MAP_STACK [umd155924/286116 sp=7eac5a1ff060 inside 7b88bcb79000-7b88b4378fff: not MAP_STACK Not sure if this is related to starting of the VMs or something else, the ToR node was consuming 100%+ CPU at the time. :) Mischa I have not seen this; can you try without the ToR node some time and see if this still happens? Testing again without any other VMs running. Things wrong when I run the following command and wait a little. for i in $(jot 10 10); do vmctl create -b /var/vmm/vm09.qcow2 /var/vmm/vm${i}.qcow2 && vmctl start -L -d /var/vmm/vm${i}.qcow2 -m 2G vm${i}; done Can you try adding a "sleep 2" or something in the loop? I can't think of a reason my changes would cause this. Do you see this on -current without the diff? Adding the sleep 2 does indeed help. I managed to get 20 VMs started this way, before it would choke on 2-3. Do I only need the unpatched kernel or also the vmd/vmctl from snap? Mischa
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
On 2023-09-04 18:55, Mischa wrote: On 2023-09-04 17:57, Dave Voutila wrote: Mischa writes: On 2023-09-04 16:23, Mike Larkin wrote: On Mon, Sep 04, 2023 at 02:30:23PM +0200, Mischa wrote: On 2023-09-03 21:18, Dave Voutila wrote: > Mischa writes: > > > Nice!! Thanx Dave! > > > > Running go brrr as we speak. > > Testing with someone who is running Debian. > > Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time > unless I hear of any issues. There are a couple of permanent VMs running on this host, 1 ToR node, OpenBSD VM and a Debian VM. While they were running I started my stress script. The first round I started 40 VMs with just bsd.rd, 2G memory All good, then I started 40 VMs with a base disk and 2G memory. After 20 VMs started I got the following messages on the console: [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: not MAP_STACK [umd159360/355276 sp=783369$96750 inside 7256d538c000-725645b8bFff: not MAP_STACK [umd172263/319211 sp=70fb86794b60 inside 75247a4d2000-75247acdifff: not MAP_STACK [umd142824/38950 sp=7db1ed2a64d0 inside 756c57d18000-756c58517fff: not MAP_STACK [umd19808/286658 sp=7dbied2a64d0 inside 70f685f41000-70f6867dofff: not MAP_STACK [umd193279/488634 sp=72652c3e3da0 inside 7845f168d000-7845f1e8cfff: not MAP_STACK [umd155924/286116 sp=7eac5a1ff060 inside 7b88bcb79000-7b88b4378fff: not MAP_STACK Not sure if this is related to starting of the VMs or something else, the ToR node was consuming 100%+ CPU at the time. :) Mischa I have not seen this; can you try without the ToR node some time and see if this still happens? Testing again without any other VMs running. Things wrong when I run the following command and wait a little. for i in $(jot 10 10); do vmctl create -b /var/vmm/vm09.qcow2 /var/vmm/vm${i}.qcow2 && vmctl start -L -d /var/vmm/vm${i}.qcow2 -m 2G vm${i}; done Can you try adding a "sleep 2" or something in the loop? I can't think of a reason my changes would cause this. Do you see this on -current without the diff? Adding the sleep 2 does indeed help. I managed to get 20 VMs started this way, before it would choke on 2-3. Do I only need the unpatched kernel or also the vmd/vmctl from snap? I do still get the same message on the console, but the machine isn't freezing up. [umd173152/210775 sp=7a5f577a1780 inside 702698535000-702698d34fff: not MAP_STACK Mischa
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
On 2023-09-04 18:58, Mischa wrote: On 2023-09-04 18:55, Mischa wrote: On 2023-09-04 17:57, Dave Voutila wrote: Mischa writes: On 2023-09-04 16:23, Mike Larkin wrote: On Mon, Sep 04, 2023 at 02:30:23PM +0200, Mischa wrote: On 2023-09-03 21:18, Dave Voutila wrote: > Mischa writes: > > > Nice!! Thanx Dave! > > > > Running go brrr as we speak. > > Testing with someone who is running Debian. > > Great. I'll plan on committing this tomorrow afternoon (4 Sep) my time > unless I hear of any issues. There are a couple of permanent VMs running on this host, 1 ToR node, OpenBSD VM and a Debian VM. While they were running I started my stress script. The first round I started 40 VMs with just bsd.rd, 2G memory All good, then I started 40 VMs with a base disk and 2G memory. After 20 VMs started I got the following messages on the console: [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: not MAP_STACK [umd159360/355276 sp=783369$96750 inside 7256d538c000-725645b8bFff: not MAP_STACK [umd172263/319211 sp=70fb86794b60 inside 75247a4d2000-75247acdifff: not MAP_STACK [umd142824/38950 sp=7db1ed2a64d0 inside 756c57d18000-756c58517fff: not MAP_STACK [umd19808/286658 sp=7dbied2a64d0 inside 70f685f41000-70f6867dofff: not MAP_STACK [umd193279/488634 sp=72652c3e3da0 inside 7845f168d000-7845f1e8cfff: not MAP_STACK [umd155924/286116 sp=7eac5a1ff060 inside 7b88bcb79000-7b88b4378fff: not MAP_STACK Not sure if this is related to starting of the VMs or something else, the ToR node was consuming 100%+ CPU at the time. :) Mischa I have not seen this; can you try without the ToR node some time and see if this still happens? Testing again without any other VMs running. Things wrong when I run the following command and wait a little. for i in $(jot 10 10); do vmctl create -b /var/vmm/vm09.qcow2 /var/vmm/vm${i}.qcow2 && vmctl start -L -d /var/vmm/vm${i}.qcow2 -m 2G vm${i}; done Can you try adding a "sleep 2" or something in the loop? I can't think of a reason my changes would cause this. Do you see this on -current without the diff? Adding the sleep 2 does indeed help. I managed to get 20 VMs started this way, before it would choke on 2-3. Do I only need the unpatched kernel or also the vmd/vmctl from snap? I do still get the same message on the console, but the machine isn't freezing up. [umd173152/210775 sp=7a5f577a1780 inside 702698535000-702698d34fff: not MAP_STACK Starting 30 VMs this way caused the machine to become unresponsive again, but nothing on the console. :( Mischa
Re: vmd/vmm: remove an ioctl from the vcpu hotpath, go brrr
On Mon, Sep 04, 2023 at 07:57:18PM +0200, Mischa wrote: > On 2023-09-04 18:58, Mischa wrote: > > On 2023-09-04 18:55, Mischa wrote: > > > On 2023-09-04 17:57, Dave Voutila wrote: > > > > Mischa writes: > > > > > On 2023-09-04 16:23, Mike Larkin wrote: > > > > > > On Mon, Sep 04, 2023 at 02:30:23PM +0200, Mischa wrote: > > > > > > > On 2023-09-03 21:18, Dave Voutila wrote: > > > > > > > > Mischa writes: > > > > > > > > > > > > > > > > > Nice!! Thanx Dave! > > > > > > > > > > > > > > > > > > Running go brrr as we speak. > > > > > > > > > Testing with someone who is running Debian. > > > > > > > > > > > > > > > > Great. I'll plan on committing this tomorrow afternoon (4 Sep) > > > > > > > > my time > > > > > > > > unless I hear of any issues. > > > > > > > There are a couple of permanent VMs running on this host, 1 ToR > > > > > > > node, > > > > > > > OpenBSD VM and a Debian VM. > > > > > > > While they were running I started my stress script. > > > > > > > The first round I started 40 VMs with just bsd.rd, 2G memory > > > > > > > All good, then I started 40 VMs with a base disk and 2G memory. > > > > > > > After 20 VMs started I got the following messages on the console: > > > > > > > [umd116390/221323 sp=752d7ac9f090 inside 75c264948000-75c26147fff: > > > > > > > not > > > > > > > MAP_STACK > > > > > > > [umd159360/355276 sp=783369$96750 inside > > > > > > > 7256d538c000-725645b8bFff: > > > > > > > not > > > > > > > MAP_STACK > > > > > > > [umd172263/319211 sp=70fb86794b60 inside > > > > > > > 75247a4d2000-75247acdifff: > > > > > > > not > > > > > > > MAP_STACK > > > > > > > [umd142824/38950 sp=7db1ed2a64d0 inside > > > > > > > 756c57d18000-756c58517fff: not > > > > > > > MAP_STACK > > > > > > > [umd19808/286658 sp=7dbied2a64d0 inside > > > > > > > 70f685f41000-70f6867dofff: not > > > > > > > MAP_STACK > > > > > > > [umd193279/488634 sp=72652c3e3da0 inside > > > > > > > 7845f168d000-7845f1e8cfff: > > > > > > > not > > > > > > > MAP_STACK > > > > > > > [umd155924/286116 sp=7eac5a1ff060 inside > > > > > > > 7b88bcb79000-7b88b4378fff: > > > > > > > not > > > > > > > MAP_STACK > > > > > > > Not sure if this is related to starting of the VMs or something > > > > > > > else, the > > > > > > > ToR node was consuming 100%+ CPU at the time. :) > > > > > > > Mischa > > > > > > I have not seen this; can you try without the ToR node > > > > > > some time and > > > > > > see if > > > > > > this still happens? > > > > > > > > > > Testing again without any other VMs running. > > > > > Things wrong when I run the following command and wait a little. > > > > > > > > > > for i in $(jot 10 10); do vmctl create -b /var/vmm/vm09.qcow2 > > > > > /var/vmm/vm${i}.qcow2 && vmctl start -L -d > > > > > /var/vmm/vm${i}.qcow2 -m 2G > > > > > vm${i}; done > > > > > > > > Can you try adding a "sleep 2" or something in the loop? I can't > > > > think > > > > of a reason my changes would cause this. Do you see this on -current > > > > without the diff? > > > > > > Adding the sleep 2 does indeed help. I managed to get 20 VMs started > > > this way, before it would choke on 2-3. > > > > > > Do I only need the unpatched kernel or also the vmd/vmctl from snap? > > > > I do still get the same message on the console, but the machine isn't > > freezing up. > > > > [umd173152/210775 sp=7a5f577a1780 inside 702698535000-702698d34fff: not > > MAP_STACK > > Starting 30 VMs this way caused the machine to become unresponsive again, > but nothing on the console. :( > > Mischa Were you seeing these uvm errors before this diff? If so, this isn't causing the problem and something else is. If this diff causes the errors to occur, and without the diff it's fine, then we need to look into that. Also I think a pid number in that printf might be useful, I'll see what I can find. If it's not vmd causing this and rather some other process then that would be good to know also.
Re: tcp sync cache signed use counter
> On 4 Sep 2023, at 19:52, Alexander Bluhm wrote: > > On Mon, Sep 04, 2023 at 07:22:03PM +0300, Vitaliy Makkoveev wrote: >>> On 4 Sep 2023, at 16:19, Alexander Bluhm wrote: >>> >>> Hi, >>> >>> Variable scs_use is basically counting packet insertions to syn >>> cache, so I would prefer type long to exclude overflow on fast >>> machines. With the current limits int should be enough, but long >>> does not hurt. >>> >>> It can be negative as it starts at a positive limit and counts >>> backwards. After all entries in the current syn cache have been >>> timed out, it is reset to positive limit. If timeout takes a while, >>> it may get well below zero. >>> >>> To prevent netstat output like this >>> 18446744073709531826 uses of current SYN cache left >>> make tcps_sc_uses_left signed and print it as long long integer. >>> >>> ok? >>> >> >> Does the negative value output makes sense? If not, could you >> prevent negative value output too? > > Goal is to reseed the hash algorithm from time to time. This cannot > be done while there are still entries in the hash table. > > So we have two syn caches, an active and an inactive one. > > They switch roles when the active syn cache has been used for a > while and the inactive one is empty. The scs_use counts the packets > that have been inserted in the active one. > > We start at a limit that has been set by sysctl. We count down to > 0, then use the inactive one. But it may be busy, so we wait until > all entries have timed out. While that the active one is still > used, and the counter gets negative. > > Negative numbers express how long the active syn cache has been > used above the limit. > > Of course I could move the range to have positive numbers only. > But then the state "used above limit" would not be obvious anymore. > Thanks for explanation. No objections from me. > >>> Index: sys/netinet/tcp_var.h >>> === >>> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v >>> retrieving revision 1.170 >>> diff -u -p -r1.170 tcp_var.h >>> --- sys/netinet/tcp_var.h 28 Aug 2023 14:50:02 - 1.170 >>> +++ sys/netinet/tcp_var.h 4 Sep 2023 13:02:40 - >>> @@ -288,9 +288,9 @@ struct syn_cache_head { >>> >>> struct syn_cache_set { >>> struct syn_cache_head *scs_buckethead; >>> + longscs_use; >>> int scs_size; >>> int scs_count; >>> - int scs_use; >>> u_int32_t scs_random[5]; >>> }; >>> >>> @@ -430,7 +430,7 @@ struct tcpstat { >>> u_int64_t tcps_sc_entry_limit; /* limit of syn cache entries */ >>> u_int64_t tcps_sc_bucket_maxlen;/* maximum # of entries in any bucket */ >>> u_int64_t tcps_sc_bucket_limit; /* limit of syn cache bucket list */ >>> - u_int64_t tcps_sc_uses_left;/* use counter of current syn cache */ >>> + int64_t tcps_sc_uses_left; /* use counter of current syn cache */ >>> >>> u_int64_t tcps_conndrained; /* # of connections drained */ >>> >>> Index: usr.bin/netstat/inet.c >>> === >>> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v >>> retrieving revision 1.178 >>> diff -u -p -r1.178 inet.c >>> --- usr.bin/netstat/inet.c 7 Jul 2023 09:15:13 - 1.178 >>> +++ usr.bin/netstat/inet.c 4 Sep 2023 12:51:01 - >>> @@ -498,7 +498,7 @@ tcp_stats(char *name) >>> "\t%llu entr%s in current SYN cache, limit is %llu\n"); >>> p2b(tcps_sc_bucket_maxlen, tcps_sc_bucket_limit, >>> "\t%llu longest bucket length in current SYN cache, limit is >>> %llu\n"); >>> - p(tcps_sc_uses_left, "\t%llu use%s of current SYN cache left\n"); >>> + p(tcps_sc_uses_left, "\t%lld use%s of current SYN cache left\n"); >>> >>> p(tcps_sack_recovery_episode, "\t%llu SACK recovery episode%s\n"); >>> p(tcps_sack_rexmits,
Re: tcp sync cache signed use counter
Alexander Bluhm: > Variable scs_use is basically counting packet insertions to syn > cache, so I would prefer type long to exclude overflow on fast > machines. With the current limits int should be enough, but long > does not hurt. But long is the same size as int. On our 32-bit archs. Or are those inherently so slow that this doesn't matter for them? -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: tcp sync cache signed use counter
> On 4 Sep 2023, at 23:43, Christian Weisgerber wrote: > > Alexander Bluhm: > >> Variable scs_use is basically counting packet insertions to syn >> cache, so I would prefer type long to exclude overflow on fast >> machines. With the current limits int should be enough, but long >> does not hurt. > > But long is the same size as int. On our 32-bit archs. Or are > those inherently so slow that this doesn't matter for them? > Guess this relies to packets count. 32-bit machines are much limited, so a 32-bit counter is sufficient, while 64-bit machines can store many more packets.
Re: unifdef HAS_INLINES in make(1)
On Tue, Sep 05, 2023 at 12:04:24AM +1000, Jonathan Gray wrote: > inline is part of gnu89 and c99 > > Index: defines.h > === > RCS file: /cvs/src/usr.bin/make/defines.h,v > retrieving revision 1.15 > diff -u -p -r1.15 defines.h > --- defines.h 14 Oct 2015 13:50:22 - 1.15 > +++ defines.h 4 Sep 2023 13:49:16 - > @@ -61,16 +61,8 @@ typedef struct Suff_ Suff; > > #ifdef __GNUC__ > # define UNUSED __attribute__((__unused__)) > -# define HAS_INLINES > -# define INLINE __inline__ > #else > # define UNUSED > -#endif > - > -#ifdef HAS_INLINES > -# ifndef INLINE > -# define INLINE inline > -# endif > #endif > > /* > Index: lst.h > === > RCS file: /cvs/src/usr.bin/make/lst.h,v > retrieving revision 1.33 > diff -u -p -r1.33 lst.h > --- lst.h 4 Mar 2021 09:45:31 - 1.33 > +++ lst.h 4 Sep 2023 13:49:52 - > @@ -159,25 +159,16 @@ extern void * Lst_DeQueue(Lst); > #define Lst_Adv(ln) ((ln)->nextPtr) > #define Lst_Rev(ln) ((ln)->prevPtr) > > - > -/* Inlines are preferable to macros here because of the type checking. */ > -#ifdef HAS_INLINES > -static INLINE LstNode > +static inline LstNode > Lst_FindConst(Lst l, FindProcConst cProc, const void *d) > { > return Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d); > } > > -static INLINE LstNode > +static inline LstNode > Lst_FindFromConst(LstNode ln, FindProcConst cProc, const void *d) > { > return Lst_FindFrom(ln, (FindProc)cProc, (void *)d); > } > -#else > -#define Lst_FindConst(l, cProc, d) \ > - Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d) > -#define Lst_FindFromConst(ln, cProc, d) \ > - Lst_FindFrom(ln, (FindProc)cProc, (void *)d) > -#endif > > #endif /* _LST_H_ */ > Index: lst.lib/lst.h > === > RCS file: /cvs/src/usr.bin/make/lst.lib/lst.h,v > retrieving revision 1.2 > diff -u -p -r1.2 lst.h > --- lst.lib/lst.h 14 Oct 2015 13:52:11 - 1.2 > +++ lst.lib/lst.h 4 Sep 2023 13:50:30 - > @@ -154,25 +154,16 @@ extern void * Lst_DeQueue(Lst); > #define Lst_Adv(ln) ((ln)->nextPtr) > #define Lst_Rev(ln) ((ln)->prevPtr) > > - > -/* Inlines are preferable to macros here because of the type checking. */ > -#ifdef HAS_INLINES > -static INLINE LstNode > +static inline LstNode > Lst_FindConst(Lst l, FindProcConst cProc, const void *d) > { > return Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d); > } > > -static INLINE LstNode > +static inline LstNode > Lst_FindFromConst(LstNode ln, FindProcConst cProc, const void *d) > { > return Lst_FindFrom(ln, (FindProc)cProc, (void *)d); > } > -#else > -#define Lst_FindConst(l, cProc, d) \ > - Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d) > -#define Lst_FindFromConst(ln, cProc, d) \ > - Lst_FindFrom(ln, (FindProc)cProc, (void *)d) > -#endif > > #endif /* _LST_H_ */ > > Please Cc: me on make stuff, I don't always see all mails. Have you checked it works on gcc3 or gotten someone to check ? If so, it's okay.
installer: support encryption with key disks
Extend the yes/no question to no/passphrase/keydisk and have users pick an existing, preformated RAID partition; no support (yet) for creating one. Thanks to how ask_which() works, users can always say 'done' to land back at question to either skip crypto or use a passphrase instead. All code remains contained behind interactive non-default installations. Code is straight forward, I've not been able to break it; rest unchanged. Example install with root disk sd0 and ready-to-use key disk sd1: Available disks are: sd0 sd1. Which disk is the root disk? ('?' for details) [sd0] Encrypt the root disk with a (p)assphrase or (k)eydisk? [no] k Available disks are: sd1. Which disk contains the key disk? (or 'done') [sd1] Available sd1 partitions are: a. Which sd1 partition is the key disk? (or 'done') [a] Configuring the crypto chunk sd0... No valid MBR or GPT. Use (W)hole disk MBR, whole disk (G)PT or (E)dit? [whole] Setting OpenBSD MBR partition to whole sd0...done. sd2 at scsibus2 targ 1 lun 0: sd2: 1023MB, 512 bytes/sector, 2096560 sectors Configuring the root disk sd2... No valid MBR or GPT. Use (W)hole disk MBR, whole disk (G)PT or (E)dit? [whole] Feedback? OK? Index: install.sub === RCS file: /cvs/src/distrib/miniroot/install.sub,v retrieving revision 1.1255 diff -u -p -r1.1255 install.sub --- install.sub 21 Aug 2023 14:33:55 - 1.1255 +++ install.sub 4 Sep 2023 21:32:14 - @@ -3074,8 +3074,32 @@ do_autoinstall() { exec reboot } +# Chose an existing partition as key disk and set global $KEYDISK on success, +# otherwise return non-zero. +pick_keydisk() { + KEYDISK= + local _disk _label + + ask_which disk 'contains the key disk' '$(rmel $ROOTDISK $(get_dkdevs))' + [[ $resp == done ]] && return 1 + _disk=$resp + + make_dev $_disk + if disklabel $_disk 2>/dev/null | ! grep -qw RAID; then + echo "$_disk must contain a RAID partition." + return 1 + fi + +ask_which "$_disk partition" 'is the key disk' \ + "\$(disklabel $_disk 2>/dev/null | + sed -En 's/^ ([a-p]):.*RAID.*$/\1/p')" + [[ $resp == done ]] && return 1 + _label=$resp + KEYDISK=$_disk$_label +} + encrypt_root() { - local _chunk=$ROOTDISK + local _args _chunk=$ROOTDISK [[ $MDBOOTSR == y ]] || return @@ -3088,13 +3112,30 @@ encrypt_root() { # e.g. auto-assembled at boot or done in (S)hell. [[ -z $(get_softraid_volumes) ]] || return - ask_yn 'Encrypt the root disk with a passphrase?' || return + while :; do + ask 'Encrypt the root disk with a (p)assphrase or (k)eydisk?' no + case $resp in + # Retry on failure to allow passphrase or skip. + [kK]*) + pick_keydisk || continue + _args=-k$KEYDISK + break + ;; + # Do nothing, bioctl(8) will handle the passphrase. + [pP]*) break + ;; + [nN]*) return + ;; + *) echo "'$resp' is not a valid choice." + ;; + esac + done echo "\nConfiguring the crypto chunk $_chunk...\n" md_prep_fdisk $_chunk echo 'RAID *' | disklabel -w -A -T- $_chunk - bioctl -Cforce -cC -l${_chunk}a softraid0 >/dev/null + bioctl -Cforce -cC -l${_chunk}a $_args softraid0 >/dev/null # No volumes existed before asking, but we just created one. ROOTDISK=$(get_softraid_volumes)
Re: clockintr: move control of statclock() into scheduler
On Thu, Aug 31, 2023 at 04:01:35PM -0500, Scott Cheloha wrote: > This is the next patch in the clock interrupt policy reorganization > series. > > While the hardclock/dt(4) patch is being rewritten we can do this > orthogonal statclock() patch. It needs to get done at some point > anyway, may as well do it now. > > So, this patch moves most of the statclock() code out of the clockintr > layer and cedes control of the statclock() to the scheduler. My thinking > is: (a) statclock() increments p_cpticks and calls schedclock(), so in > practice it is a scheduler interrupt, and (b) in the future it would be > nice if the scheduler could disable the statclock when a CPU is very idle > and maybe save some power. > > All of this should feel familiar. It is equivalent to what we just > did to roundrobin(). > > - Move the contents of the clockintr_statclock() wrapper function > into statclock() and make statclock() a real clockintr callback. > > - clockintr_expiration(), clockintr_nsecuptime(), and > clockintr_schedule() all become public sys/clockintr.h > interfaces for use in statclock(). > > - Tweak statclock() to handle multiple expirations at once. > > - Move the statclock handle from clockintr_queue (cq_statclock) to > schedstate_percpu (spc_statclock). Establish spc_statclock during > sched_init_cpu(). > > - Move the statclock variables from kern_clockintr.c to kern_clock.c. > Move statclock variable initialization from clockintr_init() forward > into initclocks(). > > - Replace the CL_RNDSTAT flag with a new global boolean, > statclock_is_randomized. Update clockintr_init() callers to set > statclock_is_randomized instead of passing CL_RNDSTAT. Ping. ok? Index: kern/kern_clock.c === RCS file: /cvs/src/sys/kern/kern_clock.c,v retrieving revision 1.115 diff -u -p -r1.115 kern_clock.c --- kern/kern_clock.c 23 Aug 2023 01:55:45 - 1.115 +++ kern/kern_clock.c 5 Sep 2023 01:13:22 - @@ -39,6 +39,7 @@ #include #include +#include #include #include #include @@ -87,17 +88,42 @@ int ticks = INT_MAX - (15 * 60 * HZ); /* Don't force early wrap around, triggers bug in inteldrm */ volatile unsigned long jiffies; +uint32_t statclock_avg;/* [I] average statclock period (ns) */ +uint32_t statclock_min;/* [I] minimum statclock period (ns) */ +uint32_t statclock_mask; /* [I] set of statclock_min offsets */ +int statclock_is_randomized; /* [I] fixed or pseudo-random period */ + /* * Initialize clock frequencies and start both clocks running. */ void initclocks(void) { + uint32_t half_avg, var; + /* * Let the machine-specific code do its bit. */ cpu_initclocks(); + KASSERT(stathz >= 1 && stathz <= 10); + + /* +* Compute the average statclock() period. Then find var, the +* largest power of two such that var <= statclock_avg / 2. +*/ + statclock_avg = 10 / stathz; + half_avg = statclock_avg / 2; + for (var = 1U << 31; var > half_avg; var /= 2) + continue; + + /* +* Set a lower bound for the range using statclock_avg and var. +* The mask for that range is just (var - 1). +*/ + statclock_min = statclock_avg - (var / 2); + statclock_mask = var - 1; + KASSERT(profhz >= stathz && profhz <= 10); KASSERT(profhz % stathz == 0); profclock_period = 10 / profhz; @@ -246,12 +272,30 @@ stopprofclock(struct process *pr) * do process and kernel statistics. */ void -statclock(struct clockframe *frame) +statclock(struct clockintr *cl, void *cf) { + uint64_t count, expiration, i, uptime; + struct clockframe *frame = cf; struct cpu_info *ci = curcpu(); struct schedstate_percpu *spc = &ci->ci_schedstate; struct proc *p = curproc; struct process *pr; + uint32_t off; + + if (statclock_is_randomized) { + count = 0; + expiration = clockintr_expiration(cl); + uptime = clockintr_nsecuptime(cl); + while (expiration <= uptime) { + while ((off = (random() & statclock_mask)) == 0) + continue; + expiration += statclock_min + off; + count++; + } + clockintr_schedule(cl, expiration); + } else { + count = clockintr_advance(cl, statclock_avg); + } if (CLKF_USERMODE(frame)) { pr = p->p_p; @@ -259,11 +303,11 @@ statclock(struct clockframe *frame) * Came from user mode; CPU was in user state. * If this process is being profiled record the tick. */ - p->p_uticks++; + p->p_uticks += count; if (pr->ps_nice >
Re: unifdef HAS_INLINES in make(1)
On Mon, Sep 04, 2023 at 11:51:33PM +0200, Marc Espie wrote: > On Tue, Sep 05, 2023 at 12:04:24AM +1000, Jonathan Gray wrote: > > inline is part of gnu89 and c99 > > > > Index: defines.h > > === > > RCS file: /cvs/src/usr.bin/make/defines.h,v > > retrieving revision 1.15 > > diff -u -p -r1.15 defines.h > > --- defines.h 14 Oct 2015 13:50:22 - 1.15 > > +++ defines.h 4 Sep 2023 13:49:16 - > > @@ -61,16 +61,8 @@ typedef struct Suff_ Suff; > > > > #ifdef __GNUC__ > > # define UNUSED__attribute__((__unused__)) > > -# define HAS_INLINES > > -# define INLINE __inline__ > > #else > > # define UNUSED > > -#endif > > - > > -#ifdef HAS_INLINES > > -# ifndef INLINE > > -# define INLINE inline > > -# endif > > #endif > > > > /* > > Index: lst.h > > === > > RCS file: /cvs/src/usr.bin/make/lst.h,v > > retrieving revision 1.33 > > diff -u -p -r1.33 lst.h > > --- lst.h 4 Mar 2021 09:45:31 - 1.33 > > +++ lst.h 4 Sep 2023 13:49:52 - > > @@ -159,25 +159,16 @@ extern void * Lst_DeQueue(Lst); > > #define Lst_Adv(ln)((ln)->nextPtr) > > #define Lst_Rev(ln)((ln)->prevPtr) > > > > - > > -/* Inlines are preferable to macros here because of the type checking. */ > > -#ifdef HAS_INLINES > > -static INLINE LstNode > > +static inline LstNode > > Lst_FindConst(Lst l, FindProcConst cProc, const void *d) > > { > > return Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d); > > } > > > > -static INLINE LstNode > > +static inline LstNode > > Lst_FindFromConst(LstNode ln, FindProcConst cProc, const void *d) > > { > > return Lst_FindFrom(ln, (FindProc)cProc, (void *)d); > > } > > -#else > > -#define Lst_FindConst(l, cProc, d) \ > > - Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d) > > -#define Lst_FindFromConst(ln, cProc, d) \ > > - Lst_FindFrom(ln, (FindProc)cProc, (void *)d) > > -#endif > > > > #endif /* _LST_H_ */ > > Index: lst.lib/lst.h > > === > > RCS file: /cvs/src/usr.bin/make/lst.lib/lst.h,v > > retrieving revision 1.2 > > diff -u -p -r1.2 lst.h > > --- lst.lib/lst.h 14 Oct 2015 13:52:11 - 1.2 > > +++ lst.lib/lst.h 4 Sep 2023 13:50:30 - > > @@ -154,25 +154,16 @@ extern void * Lst_DeQueue(Lst); > > #define Lst_Adv(ln)((ln)->nextPtr) > > #define Lst_Rev(ln)((ln)->prevPtr) > > > > - > > -/* Inlines are preferable to macros here because of the type checking. */ > > -#ifdef HAS_INLINES > > -static INLINE LstNode > > +static inline LstNode > > Lst_FindConst(Lst l, FindProcConst cProc, const void *d) > > { > > return Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d); > > } > > > > -static INLINE LstNode > > +static inline LstNode > > Lst_FindFromConst(LstNode ln, FindProcConst cProc, const void *d) > > { > > return Lst_FindFrom(ln, (FindProc)cProc, (void *)d); > > } > > -#else > > -#define Lst_FindConst(l, cProc, d) \ > > - Lst_FindFrom(Lst_First(l), (FindProc)cProc, (void *)d) > > -#define Lst_FindFromConst(ln, cProc, d) \ > > - Lst_FindFrom(ln, (FindProc)cProc, (void *)d) > > -#endif > > > > #endif /* _LST_H_ */ > > > > > Please Cc: me on make stuff, I don't always see all mails. > > Have you checked it works on gcc3 or gotten someone to check ? > If so, it's okay. No, but it must work. Used in many places, such as: sys/kern/subr_pool.c:static inline int usr.bin/sed/process.c:static inline int