All of my recent amd64 comments apply here as well.

In D26424#588892, @markj wrote:

In D26424#588887, @alc wrote:

Suppose I'm a ZFS user and I start a bhyve VM with a large guest-physical memory and the -S option. My impression is that it is not unusual for the ARC to consume greater than 20% of physical memory.

That's true, but the ARC will shrink in response to memory pressure, at least in principle. In particular, it will attempt to shrink while the global free page count is below the free target.

Suppose I'm a ZFS user and I start a bhyve VM with a large guest-physical memory and the -S option. My impression is that it is not unusual for the ARC to consume greater than 20% of physical memory.

You should also look at buf_ring_dequeue_mc(). It has similar problems. In fact, it never establishes a synchronizes-with relationship with the producer because the acquire is performed on the wrong field.

I'm trying to finish my final review of this change today.

In D26238#583120, @kib wrote:

I am not sure about the reuse of the existing MINCORE_SUPER bit for psind. It seemingly break ABI. Of course right now we do not have 1G superpage mappings in userspace, but it is natural to assume that MINCORE_SUPER indicates just superpage, so it should be set when 1G mappings appear. But since 1G psind is 2, we would have (current) MINCORE_SUPER bit clear.

I would take a "wait-and-see" approach to allocating the last available bit. Later adding that bit to the page size field isn't going to break any code written to this proposed 2-bit field.

Other than the potential overallocation in vm_reserv_startup(), I think that this is fine.

I have suggested to dougm@ that he evaluate reordering the array as a treap where the priority is based on the size of the segment.

Kostik, Mark, do you know of any machines with more segments?

I'm curious to see the output from "sysctl vm.phys_segs" on this machine.

I haven't had the time to do a careful, line-by-line review, but at a high level the approach looks okay.

Just an observation, not a request for further changes: I think that we could do better in terms of exploiting type stability on allocation of a vmspace. For example, I don't see why we explicitly set the vm_map's pmap field to NULL on deallocation and reinitialize it on allocation. Although the type of pmap may change on amd64 when recycling a vmspace, the vm_map will always be using the same storage for the pmap. Similarly, fields such as root, header.left, and header.right shouldn't require reinitialization.

I agree with Kostik. Please go ahead and commit the pipe changes.

In D26047#577952, @markj wrote:

In D26047#577927, @kib wrote:

In D26047#577926, @markj wrote:

In D26047#577923, @alc wrote:

I'll go a step further and question the point of even having a zone for the maps. Once upon a time, there were user-space "share" maps, but once those were eliminated and "regular" maps were embedded in the vmspace, supporting dynamic allocation of maps stopped being necessary.

Indeed, and if we remove support for dynamic allocation of kernel submaps I start to wonder if the whole submap mechanism can't itself be replaced by something simpler like a pair of vmem arenas for pipe and execve KVA ranges.

Then vm_fault() would need to handle detour into vmem arenas.

Why? The mapped ranges can correspond to kernel_map entries with MAP_ENTRY_NOFAULT clear. exec_map is subdivded this way at boot time. In pipe_map entries are allocated dynamically, and today I guess each entry gets a separate VM object.

I'll go a step further and question the point of even having a zone for the maps. Once upon a time, there were user-space "share" maps, but once those were eliminated and "regular" maps were embedded in the vmspace, supporting dynamic allocation of maps stopped being necessary.

I'll give the pv entry reclamation code a careful reading tomorrow.

(My questions are all unrelated to the direct purpose of the change.)

I'm convinced that replacing red-black trees with wavl trees is reasonable. There is barely even a trade-off between them. Wavl trees are either equal to or better than red-black trees in almost all respects. In the type of use case that actually favors red-black trees, i.e., no lookups, just random inserts and removes, the performance advantage for red-black trees that Doug reported was slight, about 2-3%. On the other hand, in a lookup-heavy workload the performance advantages of wavl trees were much more significant.

The current third paragraph of the summary should be the second paragraph. The current second and fourth paragraphs should be merged. Then, it's no longer necessary for the current fourth paragraph to say, "while no insertion adjusts the tree too much".

"... but the balance of the tree can start to get red-blackier." -> "but the balance of the tree can decay to that of a red-black tree."

"They have all the advantages of AVL trees ..." -> "They have the stricter balance of AVL trees ..."

Thank you.

Here is what I would suggest for quickly getting "rank-balanced" out of the way and moving on to the more important part, the properties of "WAVL" trees:

I think that you are trying to emulate the style of the original text describing red-black trees, and I think that you should break away from that style. Specifically, don't start with low-level definitional details. With red-black trees those details are so short that typical readers will stick around to the end of the text that says what they want to know in deciding to use red-black trees versus something else. For better or worse, those details are not so short for WAVL. So, you need to restructure the text to summarize the case for WAVL trees in the first paragraph, specifically, that they equal or dominate red-black trees in every dimension. In particular, you never explicitly say that WAVL insertion has the same complexity as red-black insertion both in the overall sense and in terms of rotations yet achieves better balance.

In D25483#566623, @kib wrote:

In D25483#566493, @alc wrote:

I would encourage you to create a followup patch that provides an overview of the various TLB management implementations. I've got time now to help with refining the text if you can generate a first draft.

I will do this.

Currently Peter finished testing D25510, and Mark already reviewed that patch. I will commit that first, then rebase D25483 (this one, the patches extensively conflict), and work on the overview in parallel.

I have two comments.

I would encourage you to create a followup patch that provides an overview of the various TLB management implementations. I've got time now to help with refining the text if you can generate a first draft.

On a barely related note, I wondered if you or Mark had observed whether the recently committed update to jemalloc has "fixed" the horrific anonymous memory fragmentation that occurs with applications like clang that mmap() a lot of files.

In D24652#545633, @kib wrote:

In D24652#545603, @alc wrote:

I would like to see any new mechanism by which we are creating physical superpages set the value of "m->psind" appropriately, so that operations on physical pages might exploit the knowledge that they are working with a physical superpage, regardless of whether that physical superpage was created by the reservation system or by some other means. That said, what makes this current patch "tricky" has little to do with physical memory. This patch is creating a new type of mapping with different properties than we've had before. For example, if I understood correctly, it is not demotable for purposes of changing any mapping attributes at a 4KB granularity. So, it makes perfect sense to me that the data structures that manage mappings (as opposed to physical memory) need to be able to express this difference. (And, the size of the physical page, i.e., "m->psind", does not describe those differences.)

I am saying that in this patch, setting m->psind would be a write-only operation, this data is unused because other layers must provide some additional controls anyway. And then, if setting m->psind, I get problems with reservations, which resolution I do not see as providing any value neither to this work, nor to the vm architecture. I think to resolve this problem, some notion of psind 'ownership' should be developed. For instance, as Mark proposed, m-object->flags & OBJ_COLORED might be the indication that m->psind is due to populated reservation.

In D24652#545634, @kib wrote:

In D24652#545630, @alc wrote:

In D24652#545553, @kib wrote:

In D24652#545452, @alc wrote:

For several generations of their microarchitecture, Intel's 2nd level TLB has been unified in two senses of that word: (1) it holds both 4KB and 2MB mappings and (2) it holds both data and instruction mappings. So, you don't really gain very many additional TLB entries by using 4KB and 2MB pages simultaneously. Specifically, you only gain extra entries in the 1st level data and instruction TLBs, where 4KB and 2MB entries are stored in separate structures. A result of this organization that doesn't get enough attention is the effect of competition between data and instruction mappings. In particular, suppose that you are using 2MB data mappings but 4KB instruction mappings for a relatively large program, like PostgreSQL. We've seen cases, e.g., clang, where forcing the use of 2MB instruction mappings reduced 2nd level TLB misses caused by data accesses by half.

You remember results of the PCID testing ? There were no measurable reduction of the context switch latency, although hwpmc demonstrated 10x times reduction of the TLB misses rate. (On the other hand, when PTI is enabled and PCID is used to cache kernel mode ptes, syscall entry latency does shrink by ~50%).

I do. However, I don't recall which benchmark saw the 10x reduction in TLB misses. Was it lmbench's lat_ctx? If so, then the reason why a 10x reduction in TLB misses has little effect on that benchmark's results is easy to explain: After every context switch to a process, that process sequentially reads *every* word in the malloc()ed region whose size is determined by the command line parameter to the benchmark. In other words, for every TLB miss that PCID might eliminate you are performing 512 sequential memory accesses (on a 64-bit machine). So, it shouldn't be surprising that the savings from the avoided TLB misses after a context switch are going to appear insignificant. I don't think that I was aware of this "feature" of lat_ctx when you were testing PCID.

I remember I tried different allocation sizes, including something ridiculously small. Anyway I tried to say that I do not expect to see significant changes in the target software (DPDK) due to use of largepage mappings. In particular because it would be dominated by other memory traffic, similar to what you noted about lat_ctx.

On the other hand, having official way to get physical contiguous mapping in userspace would be immediately useful for DPDK, OFED, and some other HPC consumers, from what I was told.

In D24652#545553, @kib wrote:

In D24652#545452, @alc wrote:

For several generations of their microarchitecture, Intel's 2nd level TLB has been unified in two senses of that word: (1) it holds both 4KB and 2MB mappings and (2) it holds both data and instruction mappings. So, you don't really gain very many additional TLB entries by using 4KB and 2MB pages simultaneously. Specifically, you only gain extra entries in the 1st level data and instruction TLBs, where 4KB and 2MB entries are stored in separate structures. A result of this organization that doesn't get enough attention is the effect of competition between data and instruction mappings. In particular, suppose that you are using 2MB data mappings but 4KB instruction mappings for a relatively large program, like PostgreSQL. We've seen cases, e.g., clang, where forcing the use of 2MB instruction mappings reduced 2nd level TLB misses caused by data accesses by half.

You remember results of the PCID testing ? There were no measurable reduction of the context switch latency, although hwpmc demonstrated 10x times reduction of the TLB misses rate. (On the other hand, when PTI is enabled and PCID is used to cache kernel mode ptes, syscall entry latency does shrink by ~50%).

In D24652#545459, @kib wrote:

In D24652#545448, @alc wrote:

In D24652#542784, @markj wrote:

My belief is that m->psind is not (or, should not be) specific to vm_reserv, and that anything which manages contiguous memory reservations, like hugetlb or vm_reserv, should be able to pass it as a hint to the fault handler without any special MI handling. Up until now vm_reserv is the only subsystem which provides such reservations, so there may be some coupling, but it should be fixed.

Yes, please.

This is in fact the v3.0 (or even 3.1) of the patch. After above discussion with Mark, and some on-line talk, I tried to use m->psind to communicate the level of the pte that needs to be installed by pmap_enter(). Unortunately, it causes vm_reserv subsystem to break. Most serious reason was that I cannot clear m->psind in the exitsing structure of pagers before vm_object_terminate() frees all object' pages (I cannot move vm_pager_dealloc() earlier, I tried that at mgmt cdev pager times, and it is worse than the problems it tries to solve). So there are free pages with either m->psind == 1 or worse m->psind == 2 left in the free pools.

Then either asserts in vm_reserv, like vm_reserv_populate(), fire. Or vm_fault_soft_fast() misbehaves in mysterious ways. This was patch v2.0. So I switched to half of what was discussed. What is left from the v2.0 is the rework to allow faults on the superpage mappings, and to process faults with populate.

Note that I still cannot get away from the special flag to pmap_enter() indicating that non-level 0 pte must be installed, this is true even for transparent superpages, which is the reason why we explicitly pass psind = 1. For instance, for transparent superpages, psind = 1 and non-consistent PKRU keys is only soft error, which causes both populate and fast fault cases to retract to psind = 0 ptes. But for non-transparent superpages, non-consistent PKRU means user error and must not shred large pte into smaller.

That said, I do not quite see what would m->psind > 0 for this patch give. I need special behavior at all levels:

• for vm_map, clip must not clip superenties, so vm_map_entry_t needs a flag to indicate it and avoid splitting
• for vm_fault/populate, we need to know that whole superpage entry must be installed (see above), but there are curious additional details. Consider 1G superpage, which consists of 256k struct vm_pages. If we busy all of them in pager->populate(), and then unbusy after pmap_enter(), this causes visible many seconds (close to a minute) processing of the page fault. So I coded the interface where vm_map_entry with no-clip flag (mask) only requires first page in the run to be busied.
• for pmap_enter(), special cases are PKRU and strict avoidance of demotion for pdes.

My point is that additional information besides m->psind > 0 is needed to ensure special behavior in all layers which might not (easily) get to the m (vm_map) or need to distinguish why psind is set (transparent vs. non-transparent). More, currently pmap cannot operate looking at m->psind > 0 as well, pmap_enter() needs psind argument.

We configure x86 CPUs to use cached accesses to page table structures,
and TLB miss is not too awful from performance PoV if page table itself
is cached. More, I suspect that most gain from the superpages comes from
the fact that we are able to use more TLB entries, not because one entry
can serve more VA.

In D24652#542784, @markj wrote:

My belief is that m->psind is not (or, should not be) specific to vm_reserv, and that anything which manages contiguous memory reservations, like hugetlb or vm_reserv, should be able to pass it as a hint to the fault handler without any special MI handling. Up until now vm_reserv is the only subsystem which provides such reservations, so there may be some coupling, but it should be fixed.

In D24652#542754, @markj wrote:

In D24652#542741, @kib wrote:

m->psind is tightly tied to the transparent promotion. I am not sure, my intuition strongly disagrees that transparent promotion to 1G could ever work. I think it would just cause 10-20% populated reservations to hang around never completing.

I really do not see why it is tightly coupled. Sure, vm_reserv sets it when a reservation is fully populated, but it is really only used by the fault handler. AFAIK there is no reason why a pager populate handler cannot set it either. Else why does vm_fault_populate() even try to handle m->psind != 0? The page fault handler does not set OBJ_COLORED when calling vm_pager_populate(). In fact I think Alan at one point mentioned that the populate interface was designed partly to accommodate an implementation of non-transparent superpages.

In D23895#525337, @markj wrote:

In D23895#525319, @alc wrote:

The _NOFREE reminds me that we still need a way to segregate _NOFREE allocations in physical memory. Such segregation would most likely provide contiguity inherently.

The freepool approach requires some work in order to segregate KVA allocations, I believe, but I think that should be straightforward.

The _NOFREE reminds me that we still need a way to segregate _NOFREE allocations in physical memory. Such segregation would most likely provide contiguity inherently.

Is your overall strategy to incrementally replace PMAP_ASSERT_STAGE1(pmap)'s by correct handling of stage 2 PTE?