In D22423#490985, @jeff wrote:

...

Maybe Alan has some thoughts. I feel like this set of patches is a welcome bandaid but I would've preferred to have done something more architectural to solve it better in the long run.

While I like the suggestion that we use a consistent variable naming schema, I felt like those style changes were overwhelming the functional changes. So, I asked Doug to separate the functional and stylistic changes.

In D22394#490621, @markj wrote:

In D22394#490201, @jeff wrote:

We run all of our paging threads constantly now. I would prefer not to disable creating cache buckets from pageproc unless we're in low memory situation. Even then it may be preferable to simply flush the buckets at the end of paging in that case.

The page daemon threads don't regularly allocate pages as far as I know. Maybe infrequently when unmapping a page will cause a superpage demotion and the leaf PTP is not cached in the pmap, and I believe that will only happen when the mapping was created with pmap_enter(psind==1).

Shouldn't this be closed?

While I think that this change is unquestionably correct, I would still ask Peter to test it.

The code is correct, but the summary is wrong.

Add context.

This looks like a reasonable change.

In D22229#485773, @kib wrote:

Although unlocked manipulations of the flags were removed AFAIR.

I think that we should consider adding a vm_object_allocate_flags().

Add a comment to efi_arch_enter() explaining why we don't update curpmap.

Add some comments.

No functional changes: Add two comments about memory ordering on context switches. Add a KASSERT that the given pmap is the curpmap in pmap_remove_pages(), just like amd64.

I spoke with Doug about this in person. I suggested that to him that instead of completely eliminating the check over the entire map that we try to reduce the frequency with which it occurs. In particular, I suggested that the consistency check be deferred until we are releasing the write lock on the map, so that, for example, an mprotect that coalesced multiple map entries would only do the consistency check once. In addition, I suggested that we might perform the consistency check 1 out of N times that we modify the vm map.

Tidy up PCPU_MD_FIELDS.

If you don't see any mysterious application (or kernel) crashes with the previous patch, then please test this new version. I believe that this new version could reasonably be committed, so also consider this message a request for review.

Implement a realistic ASID allocator. This should work with either 8- or 16-bit ASIDs.

I also found that this patch very slightly reduced the time for a "make -j16 buildkernel" on a 16-core machine. Specifically, the reduction was about 6 seconds out of about 4 minutes 35 seconds. I suspect that this is because a context switch by any core currently triggers a global TLB invalidation that affects all cores (as opposed to a local invalidation that just affects the context switching core).

If we are going to start setting a color by default on DEFAULT and SWAP objects, then the following code that appears in two places in vm_reserv.c needs to change. Specifically, it should test for the absence of the OBJ_ANON rather than the object being OBJT_VNODE.

/*                                                                                                                                                              
 * Would the last new reservation extend past the end of the object?                                                                                            
 */
if (first + maxpages > object->size) {
        /*                                                                                                                                                      
         * Don't allocate the last new reservation if the object is a                                                                                           
         * vnode or backed by another object that is a vnode.                                                                                                   
         */
        if (object->type == OBJT_VNODE ||
            (object->backing_object != NULL &&
            object->backing_object->type == OBJT_VNODE)) {
                if (maxpages == VM_LEVEL_0_NPAGES)
                        return (NULL);
                allocpages = minpages;
        }
        /* Speculate that the object may grow. */
}

In D21922#483132, @markj wrote:

In D21922#482478, @emaste wrote:

Unfortunately I haven't been able to get a ThunderX from packet.net this week

We have a couple of 1S ThunderX and one 2S ThunderX hosted at Sentex, coordinate with @gnn for use of them

I have access to one now, I will be testing today.

I believe that arm/arm/pmap-v4.c also needs this fix.

In D21922#481700, @andrew wrote:

I can boot with it on the dual package ThunderX in Sentex (2 x 48 cores). Unfortunately I was unable to test it as I hit an unrelated nfs locking issue.

If my recollection of the sparc64 pmap is correct, it will have the same issue.

In D22007#481438, @kib wrote:

BTW, I did tried to find a reference in the SDM vol.3 that would certainly witness that interrupts and exceptions are serialized, and failed. I know for sure that sysenter is not serializing.

Why not use PMAP_ENTER_NOSLEEP instead of introducing a new flag?

In D22007#481354, @cem wrote:

In D22007#481240, @kib wrote:

I remembered why I did not wanted to put the SFENCE instruction in pmap_activate_sw(). Description of e.g. CLFLUSHOPT (as well as e.g. AMD CLZERO) explicitly state that SFENCE is required, they do not mention serialization instructions.

Vol 2A, p.3-144:

Executions of the CLFLUSHOPT instruction are ordered with respect to fence instructions and to locked read-modify-write instructions; they are also ordered with respect to the following accesses to the cache line being invalidated: older writes and older executions of CLFLUSH. They are not ordered with respect to writes, executions of CLFLUSH that access other cache lines, or executions of CLFLUSHOPT regardless of cache line; to enforce CLFLUSHOPT ordering with any write, CLFLUSH, or CLFLUSHOPT operation, software can insert an SFENCE instruction between CLFLUSHOPT and that operation.

The "can" language in that last sentence is unclear to me. It seems possible that some other serializing instruction might be adequate, although I agree it is unclear why those others are not spelled out in the list above.
I find the CLWB language a little more precise (p. 3-149):

CLWB instruction is ordered only by store-fencing operations. For example, software can use an SFENCE, MFENCE, XCHG, or LOCK-prefixed instructions to ensure that previous stores are included in the write-back.

Ok, are serializing events not store-fencing operations? Or is Intel just providing examples of performant ways to order these operations rather than an exhaustive list?

In D22007#481175, @cem wrote:

In D22007#481143, @alc wrote:

Intel SDM states that interrupts and exceptions flush store buffers, but this is said in context of WB memory.

Which section?

Vol. 3A, Chapter 11 "MEMORY CACHE CONTROL" (and specifically for store buffers in §11.10 "STORE BUFFER").

The processor ensures that ... the contents of the store buffer are always drained to memory in the following situations:
• When an exception or interrupt is generated.

But the intro says this:

Write Combining (WC) ... If the WC buffer is partially filled, the writes may be delayed until the next occurrence of a serializing event; such as, an SFENCE or MFENCE instruction, CPUID execution, a read or write to uncached memory, an interrupt occurrence, or a LOCK instruction execution.

The list is oddly specific and does not include 'mov to control registers,' but the "such as" language and cpuid example suggest to me that setting cr3 may be sufficient (if, as Konstantin points out, we actually did so).

Intel SDM states that interrupts and exceptions flush store buffers, but this is said in context of WB memory.

I would appreciate it if folks would exercise this patch a bit, particularly on ThunderX, ThunderX2, and eMAG machines. I want know if there are issues with this patch, e.g., mysterious program crashes, before I rewrite the ASID allocator.

I'm confused as to why this is necessary. pmap_activate_sw() performs a serializing instruction, specifically, a move to cr3. And, Section 8.2.5 of Volume 3 says,

Program synchronization can also be carried out with serializing instructions (see Section 8.3). These instructions
are typically used at critical procedure or task boundaries to force completion of all previous instructions before a
jump to a new section of code or a context switch occurs. Like the I/O and locking instructions, the processor waits
until all previous instructions have been completed and all buffered writes have been drained to memory before
executing the serializing instruction.

And, the next paragraph discusses the use of fences, e.g., sfence, instead of serializing instructions, e.g., cpuid.

In D21964#480590, @pho wrote:

I ran all of the stress2 test on 63130. I found one problem, which to me looks unrelated.
https://people.freebsd.org/~pho/stress/log/quota10-2.txt

Tidy up locore.S. Specifically, allow callers of build_l1_block_pagetable to control other lower attributes besides cacheability.

Workaround Cavium erratum 27456.