In D58336#1338226, @olce wrote:

Fine, though please note I have a lot of rework of hwpstate_intel(4) incoming, in particular dropping this hybrid core detection code, so if you plan more/significant work in this area please liaise with me.

Refresh after conflict resolution for 02c440e20404 ("hwpstate_intel: Minimize ifdef for i386 build")

Comparing to sys/x86/include/bus.h and sys/dev/ntb/ntb_hw/ntb_hw_*.c, they just call the bus_space_tag_t parameter tag, and sys/x86/include/bus.h calls the bus_space_handle_t parameter bsh while ntb calls it handle. I think you could simplify those names. A verbatim copy of ntb's definitions would seem fine.

In D58318#1337993, @jrtc27 wrote:

D58301 seems a better alternative to this?

kib feedback and earlier olce comment: CPU_FOREACH

kib feedback from D58336: just provide pc_small_core on i386

In D58332#1337715, @koinec_yahoo.co.jp wrote:

Thank you very much for creating this.
I failed to notice the missing handler for i386 when I made my initial proposal. I apologize for that.

In D58316#1337420, @markj wrote:

In D58316#1337411, @rlibby wrote:

Maybe one day we should plumb it for malloc_dbg / memguard_alloc too.

Do you use memguard at all? I haven't needed to reach for it since KASAN was added.

Maybe one day we should plumb it for malloc_dbg / memguard_alloc too.

I also wonder if we should have an option to add this redzone without KASAN. We could come up with a cheap-ish debugging feature to verify the redzone with uma_junk when we don't have KASAN. Not asking for that in this patch.

Add an assertion to item_ctor() that the returned item is properly
aligned. I couldn't see any pre-existing checks which verify this.

The other thing that SMR does in this respect is it puts free buckets at the tail of uzd_buckets in zone_put_bucket. Did you think about whether you want that for KASAN also, to extend the length of time before reuse further?

Looks good.

Looks good.

Looks fine modulo the uz_flags comment and style and history nits.

In D58268#1336651, @markj wrote:

In D58268#1336646, @rlibby wrote:

BTW git arc can fetch patches from phabricator and apply them to a local tree, though of course that can fail, e.g., if the patch in phabricator needs a rebase. The following invocation will fetch and apply all of the patches in this series:

$ git arc patch -c -s -r D58272

Kind of cumbersome versus fetching a git branch, but hey...

Style nits, but logic looks good.

Just for review convenience, do you have a public git branch with the patch set?

Logic LGTM.

kib feedback

tweak comment and log message

markj feedback

whitespace fixup

So, why mess with the types (int -> bool and int -> u_long)? Strictly speaking it is not necessary and I can revert it if you prefer. I note though that there is no TUNABLE_UINT() and a comment for TUNABLE_INT() suggests not to use it. I don't know why this is, but just converting to bool and u_long seems fine anyway.

Addressing problems uncovered in testing. I had assumed that the
mechanism used by debug.witness.witness_count actually worked, but it
didn't. I added a new patch D57613 to address the problem with
witness_count by itself, and then applied that fix here too. I also
fixed an alignment problem.

Looks good. I presume you've tested it?

Abandoning this for now, though I still think it would be fine to take. I addressed some of the underlying issues with D56894 and D56946.

Also make the vnlru_read_freevnodes() call conditional.

Regarding braces... I prefer to leave it with the braces. My preference is to apply to both branches if applied to one. Style says "may" be left out. There are many examples of the balanced use, including about 10 in this file.

In D56951#1304662, @olce wrote:

Seems reasonable. Do you have any specific benchmark showing an actual improvement?