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.

Error Numeric Association. It's mean to be a unique value while dealing with a producer and transport until it reaches the fault manager.

Part of it is designed to include timestamps to help with correlation.

The timestamp values are overflowing it, so correlation will have issues with wraparound. :/

It wasn't intended to have the whole timestamp.

Anyway, I guess the solution is to just mask it to suppress the UBSAN complaints.

The value is already masked after shifts.

So it's not like changing that around to reduce ub is a problem.

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'

Masking before the shift to prevent an overflow will suppress the complaint.

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.

Alright. Thanks. :)

hmm: adam@sqlbox:/home/adam$ sudo zpool trim sqlpool

cannot trim 'hole': no such device in pool

I believe I added a slog, removed it, and re-added it

looks like this guy: openzfs/zfs #10906

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

KungFuJesus seems so.

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. :/

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... :/

I chased similar ghosts with ubsan trying to debug some stuff in nouveau on big endian

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

This is what I did locally: paste.ubuntu.com/p/WkCtjjMb7g

So far, no complaints. The other way to avoid this would be to change hrtime_t to unsigned.

It's not changing.

You can, but also the point of it is to be able to subtract values, so you're trading off negative values for underflow.

yeah, I'd figure negative timestamp values for a high resolution timespec are meaningful for that very purpose

Especially when it's all about relative measurements.

That makes sense.

Ubuntu's paste bin service requires a login. -_-

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

KungFuJesus: Yes, we have structures whose members are bitfields.

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

What versions of gcc does it impact?

looks like it was introduced in 2020: gcc.gnu.org/git/gitweb.cgi?p=gcc.gi…897655e3b92949b65a2b53e00fb3ab8ded0

It looks like this was intoduced in gcc11?

If I read correctly, it affects GCC 11.0 through GCC 12.2.

looks like they only backported to 12? At least from what I can tell with the PR's message chain

The last message suggested it is fixed in what will become GCC 12.3.

rmustacc: hah, got lucky switching to 10, I guess

I had been wondering why Gentoo has been shipping GCC snapshots. I guess this explains it.

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

everything managing to spill to the stack there several times over when there's like 31 or 32 GPRs to work with

KungFuJesus: I learned that GCC on PPC generates very suboptimal code during my adventures with fletcher4 GNU C vector code late last year.

That is one of the things that I recall observing too.

Although it had a vector lowering issue on all architectures in addition to that, plus other miscellaneous issues.

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

but yeah, still super suboptimal for load-store

It is still a WIP that I intend to revisit hopefully this quarter.

KungFuJesus: Also, to give an example, see efi_gpe_Attrs_t in usr/src/uts/common/sys/efi_partition.h.

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...

There are comments about this in the OpenZFS zstd code: github.com/openzfs/zfs/blob/3bbc260…ed17e3c/include/sys/zstd/zstd.h#L61

ryao: oof, isn't there a lot of code that relies on bit exact ordering in bitfields?

I rarely ever see them used, so I would think not, but that needs to be checked...

nouveau certainly does - the reproducer I have there is generating nv30-specific instructions

KungFuJesus: Ouch. They probably should not be using that to talk to hardware. Different compilers could reorder things in bad ways.

hmm, wonder if someone should let #mesa know

Probably.

It's ultimately part of the ABI.

my bad, they aren't: gitlab.freedesktop.org/mesa/mesa/-/…s/nouveau/nv30/nvfx_vertprog.c#L295 Looks like the "emit" function writes to 32 bit words

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. -_-

I mean, in general.

See sys/isa_defs.h and _BIT_FIELDS_HTOL and _BIT_FIELDS_LTOH:.

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

It all depends on what your interface is.

If your register definition is in bitfields, you have to extract it somehow.

There are a bunch of different approaches to bit manip and they all have tradeoffs.

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.

I am a fan of the BF64_GET() and BF64_SET() macros and friends in ZFS.

does that wrap xgetbv under the hood with x86 or something?

Sure, it's a variant of the what was BITX, and what we made type safe with sys/bitext.h.

KungFuJesus: See usr/src/uts/common/fs/zfs/sys/bitops.h

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.

Since compilers are free to reorder them.

ah, using generic macros with xor to set only the changed bits

Yes. :)

certainly portable, may not always be what you want for optimal, though

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...

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...

right, the bit test instruction is almost always what you want for the "get" on x86

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.

Also, just out of curiosity, why is illumos still using GCC instead of Clang?

because it's a fucking lot of work to change?

I was able to ask if it was ENOTIME. I guess that would be it.

s/able/about/

toomas is working on it for at least a shadow

to catch bugs

Neat.

I often use clang-tidy by hand to see if I've screwed something up

I don't doubt that, but it does seem worthwhile for license related issues, alone

pmooney, andyf: Is there a set of privs I could give myself to run bhyve?

as me

Clang is also a better compiler as far as I can tell.

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

richlowe: sorry, it's limited to root right now

fenix #12714

FEATURE 12714: want privilege for hypervisors (New)

↳ illumos.org/issues/12714

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. :/

I also need to write an IPD about resource limits for vmm reserovoir consumption and vCPU count

I think it would be useful to be able to constrain those per-zone

rather than the current hard limit of one bhyve VM in a zone at any given time

(GZ excluded, of course)

Also, while I am talking about compilers... it is a shame that this compiler is proprietary: compcert.org

Worth noting that bhyve drops a load of privs after it gets going, which is nice

I should teach propolis to do that too

that's a good answer. privs would be nice, but I understand it might get annoying in more deployed situations

When did illumos switch from KVM to byhve? I heard about it last month and it was news to me when I heard that.

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.

pmooney: crates.io/crates/illumos-priv if you get around to it. Let me know if there's anything you would like to see there

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

pmooney: propolis-server runs in the GZ or in a zone?

intended to run in a zone

And just manages 1 VM

yep

k, just making sure. Was playing around with "Cloud-Hypervisor" on Linux and was trying to confirm some things

didn't want to give up those isolation benefits

Yeah you REALLY want that second hull in place. ):

:)

. o O ( Security, it's a hull of a thing. )

[illumos-gate] 15196 libdladm: comparison of integers of different signs -- Toomas Soome <tsoome⊙mc>