This looks great to me -- thanks!

In D46268#1094187, @imp wrote:

My apologies for being so tardy to review this. I found one issue where FBCLOCK_LEAPSEC is never set anywhere, suggesting the change is incomplete. I'd expect it be set for the new TAI stuff, so I don't see how these changes could work. But perhaps I'm missing something.

In D46268#1094187, @imp wrote:

My apologies for being so tardy to review this. I found one issue where FBCLOCK_LEAPSEC is never set anywhere, suggesting the change is incomplete. I'd expect it be set for the new TAI stuff, so I don't see how these changes could work. But perhaps I'm missing something.

Add CLOCK_TAI support to timer_create(2).

Proposed commit message:

Thanks -- my commit bit has sadly expired, so I will update the diff here a last time if you are able to commit. Otherwise, I can try to reactivate it.

At the risk of derailing this thread entirely, having now stared at this whole code path for a while and being ever-more-profoundly annoyed by the whole mess, there are very few places we actually depend on CLOCK_REALTIME being UTC besides that POSIX says it is. It looks like it would be pretty easy to add a CLOCK_UTC definition and then a kernel tunable to switch CLOCK_REALTIME between CLOCK_UTC and CLOCK_TAI. Having a CLOCK_UTC, even if it is defined equal to CLOCK_REALTIME, might also help us annotate the limited number of places we actually really want specifically UTC in our own code. It's a thought anyway -- please ignore for the purposes of this review.

Added support for CLOCK_TAI to clock_nanosleep(2) and clock_getres(2), which were overlooked in the original patch. Not present in mutex timeouts, in-kernel timers, or timerfd, which seem like projects for the future.

Yeah, Linux's lack of error propagation was a surprise to me too. I have somewhat mixed feelings about EINVAL as the error code: it is supposed to be for if the clock doesn't exist rather than clocks that are not configured properly, but the TAI clock also doesn't exactly exist in some sense before the TAI-UTC offset is programmed.

Provide full-context diff.

As an aside, I tested the behavior of this code during leap seconds using this Linux test and it does the right thing, incrementing monotonically without glitches:

None of the Linux code I have seen checks the return value of clock_gettime(), since the return value on Linux is useless. The approaches I have seen taken are either hope-and-pray-the-result-is-right (the more common one) or to use some elaborate external checks (e.g. calling ntp_gettime() and checking for the TAI offset to be non-zero) to make sure that the kernel has the right TAI offset programmed in. We could conceivably add bug-for-bug compatibility and make it give UTC time *and* return EINVAL, but I doubt there is any code that really wants CLOCK_TAI to return UTC time and then suddenly jump 37 seconds later on.

I think this makes CLOCK_TAI behave rationally during a leap second.

Mention returning EINVAL before the TAI/UTC offset is known in the man page.

Thanks for spotting this!

I *think* this will be fine if the equivalent code moves to PPC nexus (where it came from in the first place long ago).

Looks good to me, assuming it works anyway.

Overtaken by events long ago.

Looks good to me. Thanks! There are probably more instances of this kind of thing lying around.

Nope, that's broken too in the same way:

This update doesn't actually work, though it does build:

Thanks, fixed 0/1->false/true.

More diff context, no functional changes.

Fix some fat-fingering and rename an argument to better reflect what it does.

Instead of adjusting parameters when generating VM images, adjust makefs to make free inodes when making free space. Relevant code copied from newfs(8).

So makefs(8), by default, creates a file system that can exactly hold the input tree (no extra space, no extra inodes, no extra anything). If we change the defaults in makefs(8), it seems like there are a few options:

I should also mention: one other path here is just to delete loader.kboot, since I'm not really sure it's useful. Even my PS3 loads the kernel directly from petitboot these days.

Thanks! This looks basically sane. I'll try to smoke-test it in the next few days; if I don't report back by Wednesday, treat this comment as approval.

Fix missing newline.

In D29068#651177, @rgrimes wrote:

This is a move in the right directions, at least this is state created by the installer.

We could also have makefs use expand_number(3) instead. Apologies for breakage!

This patch looks good to me from a technical point of view. I have no opinion on /boot/efi vs. /efi, so won't comment on the ultimate merits of the change, but you're welcome to include a "Reviewed by:" from me if this goes in.

Just committed this. I agree about 13.0 -- I was planning to merge to stable/13 after 13.0 has branched and smoke-test this in HEAD for a bit in the meantime.

Now tested and working, with UFS and ZFS, including multi-disk ZFS setups. In principle ready for commit.

Now does ZFS (including redundant setups) as well. Still not fully tested.

@imp any thoughts on the gmirror solution for ZFS? I think it solves 95% of the update issues and shouldn't be too bad to implement.

Just to be pedantic: We actually *do* have /boot as separate on some systems (e.g. recent powerpc64), but those systems are mutually exclusive with EFI systems. I have a mild preference /boot/efi, but it doesn't really matter much so long as we pick something -- the status quo is it not being mounted *at all*, so reliability of mounting or whatever is obviously a second-order issue.

In D15243#545983, @jhibbits wrote:

So, HTM is removed in PowerISA 3.1 and it's broken on POWER9. Is it even worth doing at this point? I'm inclined to abandon this.