The patch seems ok to me, but:

• In vm_pageout_reinsert_active(), we should change the call to read if (vm_batchqueue_insert(bq, m) != 0), even though it's just a stylistic change.
• Did you observe any measurable change in CPU usage with the patch? The reduction in lock contention looks good, but lockstat doesn't really reflect the cost of failed trylocks. Even with no observed difference I think this patch is the right way to go, especially if we can also demonstrate that pagequeue lock hold times don't go up at all.

In D37210#853891, @asomers wrote:

I'll try to figure out how to suppress them.

One problem I noticed is that the tests can spuriously fail with some error messages from scapy:

I'm surprised that clang didn't flag these. I've noticed it has trouble detecting uninitialized variable use in functions containing gotos, but that doesn't apply here.

The one in zap.c is easy to fix directly, but this is ok for now.

In D37452#851606, @jhb wrote:

Alternatively, could just add a new spin mutex lock for startup_cpus. (That's what I had planned to do). This isn't a critical performance path such that atomic ops is a big win over just having a small lock for this.

Is it a bug that tmpfs doesn't call swap_reserve_by_cred() when extending a file?

In D37296#851377, @andy_omniosce.org wrote:

Thanks for doing this work, I really appreciate the clean ups and being able to turn off the compiler and static analysis overrides in the illumos downstream.

As part of porting this over, I found a few remaining warnings that I have addressed there, which should probably be fixed in FreeBSD too. I'm posting them here for information, and if anyone wants to pick them up please feel free. Otherwise I should have time over Christmas to look at upstreaming them:

In D37195#851431, @asomers wrote:

@thj are we ready to commit?

In D37305#851395, @trasz wrote:

Only tangentially related, but I wonder if this constant shouldn't be defined for arm64 too?

Add a default case for port register I/O: return 0xffffffff
for unaligned reads. Also print debug messages for unaligned
reads and writes of port registers.

In D37390#850493, @corvink wrote:

In D37390#850476, @markj wrote:

So the problem is:

1. vcpu1 initiates a rendezvous with some set of vcpus
2. vcpu2 does not belong to the set, but observes vcpu_rendezvous_pending() (an unlocked read of vm->rendezvous_func) is true
3. vcpu2 calls vm_exit_rendezvous() but now vm->rendezvous_func is NULL because the rendezvous is already over, and this triggers an assertion failure

?

Why not simply remove the incorrect in assertion in vm_exit_rendezvous()? vm_handle_rendezvous() will do nothing if no rendezvous is pending. That is, it is harmless for vcpu2 to try to handle VM_EXITCODE_RENDEZVOUS. I think this situation is rare enough that we do not care about the cost of calling vm_exit_rendezvous() unnecessarily.

Yes, that's the issue. It's caused by vm_handle_ipi. If the OS sends INIT to a single cpu (what most OS do), vm_handle_ipi will initiate a rendezvous for a single vcpu. Don't know if the costs for unneccessary vm_exit_rendezvous calls are noticable for vms with high vcpu counts.

So the problem is:

1. vcpu1 initiates a rendezvous with some set of vcpus
2. vcpu2 does not belong to the set, but observes vcpu_rendezvous_pending() (an unlocked read of vm->rendezvous_func) is true
3. vcpu2 calls vm_exit_rendezvous() but now vm->rendezvous_func is NULL because the rendezvous is already over, and this triggers an assertion failure

?

Why does it need to be inline?

Implement the maximum structure size.

This is mainly intended to provide a fallback for TOE TLS which may need to use software decryption for an initial record at the start of a connection.

If I recall correctly, it wasn't always possible to interrupt the pager, so running certain ddb commands could make the debugger unusable and force a hard reset.

Should vm_set_topology() be updated to indicate that maxcpus is unimplemented? I can see why you didn't remove the parameter at least.