15:18:56 <ryao> Does anyone know what FM_EREPORT_ENA means? It is generated in fm_ena_generate_cpu(). ((gethrtime() << ENA_FMT1_TIME_SHFT) causes an overflow that trips linux's kernel UBSAN. I am trying to decide how to address this, but I am not happy with my present ideas and I suspect knowning what ENA means would help me decide on a resolution.
15:20:56 <rmustacc> Error Numeric Association. It's mean to be a unique value while dealing with a producer and transport until it reaches the fault manager.
15:21:09 <rmustacc> Part of it is designed to include timestamps to help with correlation.
15:23:32 <ryao> The timestamp values are overflowing it, so correlation will have issues with wraparound. :/
15:23:50 <rmustacc> It wasn't intended to have the whole timestamp.
15:24:02 <ryao> Anyway, I guess the solution is to just mask it to suppress the UBSAN complaints.
15:25:11 <rmustacc> The value is already masked after shifts.
15:25:37 <rmustacc> So it's not like changing that around to reduce ub is a problem.
15:27:26 <ryao> UBSAN will complain about any shift that does an implicit mask via an overflow: [ 8849.494447] left shift of 8836735852418 by 20 places cannot be represented in type 'long long int'
15:27:44 <ryao> Masking before the shift to prevent an overflow will suppress the complaint.
15:28:07 <rmustacc> Yes, I understand. My point was that moving around where the masking is doesn't change the intended effect, so it's not a big deal to move it.
15:28:34 <ryao> Alright. Thanks. :)
15:32:53 <KungFuJesus> hmm: adam@sqlbox:/home/adam$ sudo zpool trim sqlpool
15:32:53 <KungFuJesus> cannot trim 'hole': no such device in pool
15:33:12 <KungFuJesus> I believe I added a slog, removed it, and re-added it
15:34:27 <KungFuJesus> looks like this guy: https://github.com/openzfs/zfs/issues/10906
15:41:24 <KungFuJesus> Not sure if the fix for that depends on other commits or not, but it seems like an issue that's easy to hit, particularly if _any_ flash device gets removed/replaced
15:48:19 <tsoome> KungFuJesus seems so.
16:40:13 <ryao> I asked in ##c about why that is undefined and they pointed out that the standard specifies it is undefined on signed types, but not unsigned ones. I am not sure why hrtime_t is signed. I guess changing it now would be a headache. :/
16:41:08 <ryao> I had been reasoning about hrtime_t as a unsigned type until now, despite UBSAN telling me otherwise, since it makes no sense for it to be signed... :/
16:44:49 <KungFuJesus> I chased similar ghosts with ubsan trying to debug some stuff in nouveau on big endian
16:45:35 <KungFuJesus> FWIW, switching things to unsigned types for the mask had no discernible difference in behavior, but I imagine a compiler _could_ take some liberties with that wiggle room
16:46:20 <ryao> This is what I did locally: http://paste.ubuntu.com/p/WkCtjjMb7g/
16:46:59 <ryao> So far, no complaints. The other way to avoid this would be to change hrtime_t to unsigned.
16:47:42 <rmustacc> It's not changing.
16:48:12 <rmustacc> You can, but also the point of it is to be able to subtract values, so you're trading off negative values for underflow.
16:48:41 <KungFuJesus> yeah, I'd figure negative timestamp values for a high resolution timespec are meaningful for that very purpose
16:49:28 <rmustacc> Especially when it's all about relative measurements.
16:49:38 <ryao> That makes sense.
16:50:45 <ryao> Ubuntu's paste bin service requires a login. -_-
16:51:27 <KungFuJesus> Just curious, does Illumos use bit level fields in any of its data structures?  I found a pretty brutal bug in GCC with that that's maybe not being backported as far as 11
16:51:47 <ryao> http://sprunge.us/cPgdQq
16:52:34 <rmustacc> KungFuJesus: Yes, we have structures whose members are bitfields.
16:52:44 <KungFuJesus> It affects all CPUs when the store merging optimization is enabled (O2 enables this).  My minimal reproducer was on big endian powerpc, but technically all endiannesses are affected
16:53:11 <rmustacc> What versions of gcc does it impact?
16:53:19 <KungFuJesus> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108498
16:54:11 <KungFuJesus> looks like it was introduced in 2020: https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=e362a897655e3b92949b65a2b53e00fb3ab8ded0
16:54:45 <rmustacc> It looks like this was intoduced in gcc11?
16:54:46 <ryao> If I read correctly, it affects GCC 11.0 through GCC 12.2.
16:55:25 <KungFuJesus> looks like they only backported to 12?  At least from what I can tell with the PR's message chain
16:55:53 <ryao> The last message suggested it is fixed in what will become GCC 12.3.
16:56:23 <KungFuJesus> rmustacc: hah, got lucky switching to 10, I guess
16:57:11 <ryao> I had been wondering why Gentoo has been shipping GCC snapshots. I guess this explains it.
16:57:24 <KungFuJesus> They never addressed how suboptimal the code was for generating those GPU instructions compared to clang.  It definitely didn't leave a good taste in my mouth for GCC on PPC, anyway
16:58:48 <KungFuJesus> everything managing to spill to the stack there several times over when there's like 31 or 32 GPRs to work with
16:58:50 <ryao> KungFuJesus: I learned that GCC on PPC generates very suboptimal code during my adventures with fletcher4 GNU C vector code late last year.
16:59:20 <ryao> That is one of the things that I recall observing too.
16:59:39 <ryao> Although it had a vector lowering issue on all architectures in addition to that, plus other miscellaneous issues.
16:59:58 <KungFuJesus> Yeah, I followed that thread as well (I'm a commenter in there, somewhere).  To some degree it's understandable for the vectorized code path because until like Power8, there was no way to go from GPR to vector register
17:00:21 <KungFuJesus> but yeah, still super suboptimal for load-store
17:01:22 <ryao> It is still a WIP that I intend to revisit hopefully this quarter.
17:03:52 <ryao> KungFuJesus: Also, to give an example, see efi_gpe_Attrs_t in usr/src/uts/common/sys/efi_partition.h.
17:22:48 <ryao> KungFuJesus: By the way, there is another issue involving bitfields. Apparently, there are no ordering guarantees, so they cannot be used for structures that are written to disk. Whether efi_gpe_Attrs_t is written to disk is unclear. There is a function called efi_write() that is a pain to read that might write it to disk, in which case, we need to change things...
17:23:48 <ryao> There are comments about this in the OpenZFS zstd code: https://github.com/openzfs/zfs/blob/3bbc26097e53c472084d978dd37343b07ed17e3c/include/sys/zstd/zstd.h#L61
17:31:52 <KungFuJesus> ryao: oof, isn't there a lot of code that relies on bit exact ordering in bitfields?
17:32:24 <ryao> I rarely ever see them used, so I would think not, but that needs to be checked...
17:32:57 <KungFuJesus> nouveau certainly does - the reproducer I have there is generating nv30-specific instructions
17:33:34 <ryao> KungFuJesus: Ouch. They probably should not be using that to talk to hardware. Different compilers could reorder things in bad ways.
17:35:07 <KungFuJesus> hmm, wonder if someone should let #mesa know
17:35:21 <ryao> Probably.
17:37:16 <rmustacc> It's ultimately part of the ABI.
17:39:00 <KungFuJesus> my bad, they aren't: https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/gallium/drivers/nouveau/nv30/nvfx_vertprog.c#L295 Looks like the "emit" function writes to 32 bit words
17:39:02 <ryao> rmustacc: Are you referring to efi_gpe_Attrs_t? If that is the case and we really are writing it out to disk, then we need to make a serialization function to fix it. -_-
17:39:10 <rmustacc> I mean, in general.
17:39:36 <rmustacc> See sys/isa_defs.h and _BIT_FIELDS_HTOL and _BIT_FIELDS_LTOH:.
17:41:46 <KungFuJesus> given that fact though, it does seem a little silly to bother with bit packing fields at all.  Probably just creating a lot of overhead letting GCC figure out the instruction sequence to pack and extract them
17:42:25 <rmustacc> It all depends on what your interface is.
17:42:35 <rmustacc> If your register definition is in bitfields, you have to extract it somehow.
17:43:04 <rmustacc> There are a bunch of different approaches to bit manip and they all have tradeoffs.
17:43:42 <rmustacc> The approach say the nvme driver is using makes a bunch of things simpler when you have well defined fields. Things like some of the bitext experiments have different benefits.
17:44:42 <ryao> I am a fan of the BF64_GET() and BF64_SET() macros and friends in ZFS.
17:45:06 <KungFuJesus> does that wrap xgetbv under the hood with x86 or something?
17:45:16 <rmustacc> Sure, it's a variant of the what was BITX, and what we made type safe with sys/bitext.h.
17:46:16 <ryao> KungFuJesus: See usr/src/uts/common/fs/zfs/sys/bitops.h
17:47:23 <ryao> In any case, relying on the order of bitfields is undefined behavior, which will likely bite people if it is being read/written to/from either a disk or the network.
17:47:37 <ryao> Since compilers are free to reorder them.
17:48:20 <KungFuJesus> ah, using generic macros with xor to set only the changed bits
17:48:29 <ryao> Yes. :)
17:48:45 <KungFuJesus> certainly portable, may not always be what you want for optimal, though
17:48:53 <ryao> I need to review whether the EFI code is actually writing the structures with bitfields out when I have more time. efi_write() is not easy to read and I have a high priority bug to fix rigth now...
17:49:37 <ryao> KungFuJesus: Correct behavior is priority number 1. Speed is priority number 2. Compilers might also be able to optimize away inefficiency with something that commonly done...
17:50:18 <KungFuJesus> right, the bit test instruction is almost always what you want for the "get" on x86
17:50:32 <ryao> That said, these operations are not done in any critical loops as far as I know, so whether they are being done optimally is not super important.
17:51:55 <ryao> Also, just out of curiosity, why is illumos still using GCC instead of Clang?
17:52:16 <richlowe> because it's a fucking lot of work to change?
17:52:26 <ryao> I was able to ask if it was ENOTIME. I guess that would be it.
17:52:33 <ryao> s/able/about/
17:52:40 <richlowe> toomas is working on it for at least a shadow
17:52:42 <richlowe> to catch bugs
17:52:49 <ryao> Neat.
17:53:01 <richlowe> I often use clang-tidy by hand to see if I've screwed something up
17:53:03 <KungFuJesus> I don't doubt that, but it does seem worthwhile for license related issues, alone
17:53:27 <richlowe> pmooney, andyf: Is there a set of privs I could give myself to run bhyve? 
17:53:28 <richlowe> as me
17:53:33 <ryao> Clang is also a better compiler as far as I can tell.
17:55:35 <KungFuJesus> For a while that wasn't always the case, at least on x86.  It seems I've found a lot of counter-cases for that addage, lately.  GCCs usage of the k-mask registers on AVX512 is a bit too eager to move things to GPRs even when it's of no benefit to do so
17:55:46 <pmooney> richlowe: sorry, it's limited to root right now
17:55:54 <pmooney> fenix #12714
17:55:55 <fenix> FEATURE 12714: want privilege for hypervisors (New)
17:55:55 <fenix> ↳ https://www.illumos.org/issues/12714
17:56:10 <ryao> Clang has a cleaner codebase with fewer miscompilation bugs on common architectures (although I have heard bad things about its SPARC support). Unlike GCC's codebase, which makes one want to gorge out their eyes upon reading it, Clang's codebase makes heavy use of various C++ language features that make it a pain to figure out where functions are implemented. :/
17:56:26 <pmooney> I also need to write an IPD about resource limits for vmm reserovoir consumption and vCPU count
17:56:35 <pmooney> I think it would be useful to be able to constrain those per-zone
17:56:49 <pmooney> rather than the current hard limit of one bhyve VM in a zone at any given time
17:56:57 <pmooney> (GZ excluded, of course)
17:57:39 <ryao> Also, while I am talking about compilers... it is a shame that this compiler is proprietary: https://compcert.org/
17:58:05 <andyf> Worth noting that bhyve drops a load of privs after it gets going, which is nice
17:58:07 <andyf> https://paste.ec/paste/IFBYPJpL#G67Lpkti-5ke7jOTgN0+t/6Y8kQLtN2blgiFGwYm56q
17:58:55 <pmooney> I should teach propolis to do that too
17:59:31 <richlowe> that's a good answer.  privs would be nice, but I understand it might get annoying in more deployed situations
18:00:41 <ryao> When did illumos switch from KVM to byhve? I heard about it last month and it was news to me when I heard that.
18:02:25 <andyf> The initial commit of bhyve to illumos-gate was in May 2020. Most distributions still support KVM and bhyve, but bhyve is where all of the new work is going.
18:14:19 <papertigers> pmooney: https://crates.io/crates/illumos-priv if you get around to it.  Let me know if there's anything you would like to see there
18:24:54 <papertigers> glancing at that, it could use some of the additional privset wrappers for things like intersection.  Would be happy to add those things if they would be useful.  I might be the only one using this crate 
19:14:36 <Smithx10> pmooney:  propolis-server runs in the GZ or in a zone?
19:14:55 <pmooney> intended to run in a zone
19:15:09 <Smithx10> And just manages 1 VM 
19:15:13 <pmooney> yep
19:15:37 <Smithx10> k,  just making sure.  Was playing around with "Cloud-Hypervisor" on Linux and was trying to confirm some things
19:15:57 <pmooney> didn't want to give up those isolation benefits
19:26:36 <kebe_> Yeah you REALLY want that second hull in place.  ):
19:26:37 <kebe_> :)
19:54:04 <nomad> . o O ( Security, it's a hull of a thing. )
21:24:51 <gitomat> [illumos-gate] 15196 libdladm: comparison of integers of different signs -- Toomas Soome <tsoome⊙mc>