LGTM. I don't get why we don't send PC and the watch stop reason in the '?' case but it probably doesn't matter and it looks like a faithful refactor.

I don't have an objection to changing the LOG_WARNING to LOG_ERR, but I'm confused about what this actually changes. I looked at man 3 syslog and vsyslog1() to see if we printed to stderr by default for LOG_ERR but I'm not seeing that. So, what does this actually do?

A NULL check seems like a clear improvement over a crash.

In D28762#697585, @arichardson wrote:

Seems like this is needed to build stable/13 with GCC. Are you going to MFC this? If not I can cherry-pick it for you.

Will work on a providing a better semaphore first.

In D29542#662307, @mjg wrote:

I disagree with the patch.

The current mechanism is crap and should probably be retired, anyone interested in ramping up kernel memory use has plenty of other ways to do it.

LGTM, and I checked $work's tree for other references.

Change looks fine once the i386 hack is restored.

This can be done without introducing another object pointer (nobj) by resetting lobj and following the pattern elsewhere, but I found the approach with nobj to be more readable.

I guess this case is intended for when a single-ish writer is outpacing the buf daemon?

In D28821#645325, @markj wrote:

In D28821#645324, @mjg wrote:

While the patch at hand may indeed be good enough for a quick fixup, trying to review the entire ordeal suggests the mechanism is very error prone (at least to my taste). For example rm_tracker_remove may still be executing against IPIs and it contains multiple unordered stores.

I think that is ok, the ordering doesn't matter with respect to the IPI, which only does forward traversal and does not update the queue.

I have a couple questions, but the change itself looks fine.

In D28769#645024, @jhb wrote:

In D28769#644380, @rlibby wrote:

In D28769#644259, @jhb wrote:

The only thing that is kind of useful about the current declaration is that it documents that all kobj methods are expected to return int (and specifically kobj returns a default value if no method is found).

Hmm, I may be misunderstanding something here, but I think I see methods where that isn't true, that return void or pointers? For example, sys/kern/bus_if.m, child_deleted and get_dma_tag, then sys/dev/pci/pci.c, pci_child_deleted and pci_get_dma_tag (void and bus_dma_tag_t).

Yes, those functions do abuse the interface a bit, and it might be ok for the non-void case as long as the function has an explicit default method, but for methods without a default, kobj_error_method() is what gets used. I guess though that the generated BUS_CHILD_DELETED() does actually return void though, so perhaps it isn't really inherent to kobj aside from the default default.

In D28769#644259, @jhb wrote:

The only thing that is kind of useful about the current declaration is that it documents that all kobj methods are expected to return int (and specifically kobj returns a default value if no method is found).

In D28768#644214, @mm wrote:

I have merged lastest OpenZFS to main including the new "compatibility" property, you can close this now. Thanks for getting the change upstream!

Dropping, alternate solution committed as 0a4f851074a3ca74cd4859c20e7d9807b2aeca65.

In D28761#644041, @rrs wrote:

question

If we put the wrap
#ifdef TCPHPTS

#endif

Around the function do we still need __unused for gcc?

(it won't be unused of course if TCPHPTS is defined)

@freqlabs, @mm, sure thing.

I'm obviously not the maintainer but:

Patch itself looks fine.

markj feedback: man style

markj feedback: ANSI-fy while here

markj feedback:

• Rename the sysctl and make it tunable
• output a warning when input is dropped
• man page blurb

In D28676#642760, @markj wrote:

In D28676#642677, @rlibby wrote:

So, yeah, I don't want to break this kind of thing. On the other hand I want it to be possible to break out of a situation where a tool has executed some kind of command that would take forever to complete and could result in loss of the debugger. What's in the log you linked should fit in the 512 byte buffer, but the point stands if someone had a bigger script like that. One possibility could be just to default to the prioritize_control_input=0 behavior, so the break ability is only lost for large scripts. Another could be just a big bump in the buffer size (2k? 4k?). Thoughts?

So ddb does support a scripting facility, whereby you can run something like

db> script test= show pcpu; ps
db> run test
-- "show pcpu" output, "ps" output --

So there is at least a workaround for this kind of problem, though there you are limited by TOK_STRING_SIZE, I believe.

In D28676#642635, @markj wrote:

So if one were to programatically send a sequence of commands, separated by newlines, to ddb, does this change mean that after the first command begins executing, the echoed characters from subsequent commands will be interleaved with output from the first command?

In D28676#641723, @imp wrote:

Couple of comments but I don't know a lot about the character pipeline so I'm not sure how much good they will do.

imp feedback: provide a chicken switch