I like that this also moves the logging code text out of line.

Looks good to me.

In D21030#456361, @cem wrote:

In D21030#456320, @rlibby wrote:

Logic looks good but I think the terminology is a little misleading. What the flag actually tracks is the last drained byte (mostly, see below). Including "drain" in the name somehow might make it more clear.

Well, it's a little more complicated than that; it's a yes/no bit for "if the buffer is currently empty, did we drain a non-newline character last?" Unfortunately, the usual pattern for documenting flags only leaves a handful of characters, so I did what I could in the space. If you'd prefer a longer flag comment, I can wrap the line and elaborate.

Logic looks good but I think the terminology is a little misleading. What the flag actually tracks is the last drained byte (mostly, see below). Including "drain" in the name somehow might make it more clear.

In D20739#451136, @rlibby wrote:

In D20739#451133, @hselasky wrote:

Patch looks good to me. Did you run this patch, I.E. force the error case somehow?

General background is that this patch and others are fallout from applying the error injection in D20714 (in this case, injecting errors for M_NOWAIT allocs of mbufs). They have mostly been tested with "it used to panic and now it doesn't," but I will see if I can confirm having tickled the specific error paths with dtrace.

In D20739#451133, @hselasky wrote:

Patch looks good to me. Did you run this patch, I.E. force the error case somehow?

Fix up for conflict with r349369.

In D20722#450529, @markj wrote:

I don't quite understand how this can work in all cases. Consider vm_radix_node_zone for instance. When a radix node is freed, we expect all of its child pointers to have been cleared, and INVARIANTS kernels we define a dtor which verifies that. Then, when allocating a radix node we do not pass M_ZERO because we expect that the child pointers are already zeroed. With this change I think we will return trashed nodes, which violates the assumptions of the vm radix code.

Friendly ping. There's no particular rush, but I believe that the functionality here is useful. I have uncovered around a dozen bugs with it.

In D20744#448546, @markj wrote:

As a precaution I'd suggest running the tests under /usr/tests/sys/geom/class/mirror.

Here's an example of how I have been applying this. This one looks like a locking bug in an error path in in6_joingroup_locked. I'll submit a separate review for it.

I'm not qualified to review, but LGTM besides a couple nit picks.

@cem got this pushed upstream [1] and we pulled it back down with v1.3.8 in r342598.

Equivalent patch was done in r343672 by @vangyzen .

Silence warnings in code instead of disabling the warning.

@cem pointed out to me r340842 & r340843. @kib would you prefer we just
did the same to i386 as was done to amd64?

@cem is direct commit to sys/contrib/zstd okay for this?

dim feedback: use CWARNFLAGS.gcc for warning suppression

Always nice to have minus diffs, assuming it still compiles :)

Most of these changes look to be in the right direction. I don't think evaluating if a bool == true or == false is necessary. In some places parentheses have been added which are not strictly needed, I don't feel strongly about those either way. I did not evaluate for completeness.

My opinion is generally that branch predictions should be sparse. Is it really going to make any difference in setup/teardown functions, which many of the functions edited here appear to be?

Review feedback: avoid touching "superfluous" macro parens for now

mjoras feedback: parenthesization fixes

In D12753#264690, @mjoras wrote:

The superfluous parentheses stuff is sort of nitpicky, but I do feel that at least in the case where the expansion is surrounded by commas then it should be dropped since comma operators / separators have the lowest possible precedence.

In D12683#263679, @pfg wrote:

Upstream indeed seems to have disappeared but Benno Rice may know better.

In D12675#263212, @hselasky wrote:

Just give this patch a spin with clang aswell before committing. I'll handle this issue towards Mellanox.
Thank you!
--HPS

hselasky feedback: replace const vars with enum constants