CID 1395727 , fwiw

This changes keg cursor advancement behavior slightly. I'm not sure that matters.

Looks like kmem_back() can now be entirely removed.

ifdef _KERNEL failpoint sites

Request for acceptance into head during freeze

👍

Looks great to me.

Functionality looks good to me. Some minor style suggestions follow.

In D17056#364904, @naito.yuichiro_gmail.com wrote:

I found one problem that sshd fails to reverse resolve hostname if server is set UseDNS yes .

Looks great to me, thanks. Any other reviewers want to take a pass?

In D17083#364500, @ed wrote:

I did some tests, but C++ is something we don't need to worry about, right?

+1 to the general idea from me, and thanks Ed for taking on the work. I will review the patch later.

Hmm. I'm definitely ok rejecting the (hidden) names .sample or .pkgnew, and I think probably rejecting all hidden (dot-prefixed) names would be reasonable.

In D16335#363841, @sjg wrote:

I have no objection to and indeed welcome better names.
Sadly libveriexec is already taken.

Looks great to me! All of my comments below are just style or message suggestions, nothing functional.

Mostly looks good to me! I had a couple concerns and suggestions, see below. Thanks for the patch.

In D17056#363447, @naito.yuichiro_gmail.com wrote:

Hi Conrad.
Thanks for the advice. I regenerated patch file. It seems good differential.

I realize this is a very bikesheddy ask but would it be at all possible to make the library name longer? libveriexec or something (I don't care in particular). The vast majority of libraries have descriptive names and I don't think scrimping on a few letters is making anyone's life better.

Hi Yuichiro NAITO,

Decode signal return codes from run() more nicely in printed errors.

Removed extraneous comma, 'else's per feedback

Do any corresponding .9 man pages need updating?

Drop extra bzero of hash context, as I pointed out earlier but forgot about
:-). Thanks markm@.

Document 'thread' (entirely absent) and 'show thread' tid behavior.

In D10487#359412, @eric_metricspace.net wrote:

I'm not sure what you mean by the invariant. All this patch does is detect a violation and report an error in that case.

To provide some context in case anyone is curious:

LGTM, thanks.

Per kib, mention that devvn version of refthread correctly handles parallel
vnode reclamation.

In D16937#361440, @delphij wrote:

This would increase stack usage for /dev/random read by 32 bytes, but I think it's small enough and make the code easier to reason for MP safety (the buffer is not meant to be shared by two different threads), so consider this a LGTM from secteam@ as long as there is no objection raised by September 1st.

Update a couple CR feedback

LGTM

• Try to address CR comments
• Include in build

Perfect, looks great to me! Thanks for adding this.

Excellent! Again, thanks for taking care of this.

Changes mostly look good to me! Most of my concerns were addressed. The only outstanding issue I raised earlier but didn't feel was addressed is the bit around __packed. And just a couple new nits with the new patch below:

Thanks! Mostly looks good to me. A few quibbles:

Mostly looks good to me! Some minor doc, style, and one or two functional suggestions below.

In D16885#359928, @kib wrote:

I do not object but I must note that unconditional *ref = 1 looks strange to me. And since I am the author of that line, I believe that I added it that way for some reason which I cannot remember now. Most likely it was a compatibility issue with some third-party code. At least I looked at all in-tree callers and they looks fine.

Thanks!

In D15259#359673, @hselasky wrote:

We have this patched queued internally for testing and review. You'd like to have it in before 12 is out?

LGTM with the caveat that I am mostly unfamiliar with x86 interrupt gory details. I did not look closely at the Xen bits.

Looking very good! Some nits and doc suggestions below.

Hi Hans,

I think the improvement is conservatively correct re: my reading of Fortuna, and no need to let perfect be the enemy of good for now.

Copied from earlier review D16866:

In D16855#359281, @cem wrote:

Yeah, like I said earlier, I think that's a pretty clear typo for 255C. The temperature scale does not change between the modes and 206+49=255.

In D16855#359181, @gallatin wrote:

Thank you so much for the pointer to the docs!!! It is much nicer to write from real docs, rather than crib from undocumented changes in another project.

In D16866#359249, @markm wrote:

Split into two separate commits as per review request.

(Approved — phab won't let me mark it as such because it is "closed.")