Consider e.g. kmem_bootstrap_free(). Isn't the code there depends on the vm_page structures being already initialized?

When I worked on the pcpu + MAXCPU patch, initially I tried to reuse kmem_bootstrap_free() in some form for excess pcpu pages. This did not worked because vm_page's where not initialized, and hacks I tried to init the pages were very ugly. I am not sure how much such instances we have in the tree.

I think I would be easier accepting the approach, if for amd64 you force initialization of all vm_page's structures for pa <= 4G. Not sure what does arm64 need there, if anything.

In D40403#919776, @kib wrote:

Consider e.g. kmem_bootstrap_free(). Isn't the code there depends on the vm_page structures being already initialized?

Yes. I forgot about that function and didn't consider that anything would call vm_phys_free_pages() with an uninitialized page. I think this is not too hard to fix though.

When I worked on the pcpu + MAXCPU patch, initially I tried to reuse kmem_bootstrap_free() in some form for excess pcpu pages. This did not worked because vm_page's where not initialized, and hacks I tried to init the pages were very ugly. I am not sure how much such instances we have in the tree.

I think I would be easier accepting the approach, if for amd64 you force initialization of all vm_page's structures for pa <= 4G. Not sure what does arm64 need there, if anything.

Would you instead consider an approach where an INVARIANTS kernel can catch use-before-initialization of vm_pages? In principle this can already be done with KMSAN, but it would be better if any debug kernel can identify bugs, and we don't care as much about boot times in that case.

Initializing everything below 4GB would still hurt boot times for small VMs, and might not catch every case you're worried about.

Suppose that a kernel dump happens very early, before pages are initialized. Then, e.g. kernel memory needs the backing pages to be functional for some variations of busdma to work. I know that at very least DMAR busdma requires it.

Another question, how would the lazy initialization interact with the efi_create_1t1_map() use of VM_PHYS_TO_PAGE()? It tries to detect uninitialized page and force-init it, but at least it sounds like a race.

I still think that initing everything below 4G is the best route, if going with the lazy initialization. When/if UEFI would no longer guarantee us that everything sensitive for early startup lives below 4G. we would have no choice but extend this range.

Kboot passes through the memory map from Linux. This map is no longer 1:1 nor necessary below 4GB. The latter I'm not sure I've seen. The former always is the case, unlike what loader.efi provides. And it's impossible to change since one can only call SetVirtualMapping once and Linux already called it by the time we get control.

In D40403#920211, @kib wrote:

Suppose that a kernel dump happens very early, before pages are initialized. Then, e.g. kernel memory needs the backing pages to be functional for some variations of busdma to work. I know that at very least DMAR busdma requires it.

"kernel dump" meaning minidump? I think it is not a problem, since the minidump code always excludes free pages (they are not set in the vm_page_dump bitset), and non-free pages are always initialized.

Another question, how would the lazy initialization interact with the efi_create_1t1_map() use of VM_PHYS_TO_PAGE()? It tries to detect uninitialized page and force-init it, but at least it sounds like a race.

Only pages added to the vm_phys buddy allocator can be uninitialized after vm_page_startup(). So efi_create_1t1_map() should be fine, or else it is reinitializing pages belonging to the buddy allocator, i.e., there is already a bug.

I still think that initing everything below 4G is the best route, if going with the lazy initialization. When/if UEFI would no longer guarantee us that everything sensitive for early startup lives below 4G. we would have no choice but extend this range.

The latest revision of the patch initializes all vm_pages except for those that are initially added to the vm_phys allocator. Any pages sensitive to early startup will thus be initialized.

I am reluctant to initialize everything below 4GB: doing so eliminates the improvement for small VMs, and I cannot see any (more) concrete bugs that would be fixed as a result.

In D40403#920222, @markj wrote:

In D40403#920211, @kib wrote:

Suppose that a kernel dump happens very early, before pages are initialized. Then, e.g. kernel memory needs the backing pages to be functional for some variations of busdma to work. I know that at very least DMAR busdma requires it.

"kernel dump" meaning minidump? I think it is not a problem, since the minidump code always excludes free pages (they are not set in the vm_page_dump bitset), and non-free pages are always initialized.

I am not sure I follow. dump_add_page() is only needed for the page that are not mapped into the KVA (excluding DMAP). All active mappings except DMAP are explicitly dumped regardless of the state of the dump bitmap. How would the kernel data be dumped otherwise?

So I think a lot of pages are not initialized until later, but they are potentially dumpable.

Another question, how would the lazy initialization interact with the efi_create_1t1_map() use of VM_PHYS_TO_PAGE()? It tries to detect uninitialized page and force-init it, but at least it sounds like a race.

Only pages added to the vm_phys buddy allocator can be uninitialized after vm_page_startup(). So efi_create_1t1_map() should be fine, or else it is reinitializing pages belonging to the buddy allocator, i.e., there is already a bug.

I still think that initing everything below 4G is the best route, if going with the lazy initialization. When/if UEFI would no longer guarantee us that everything sensitive for early startup lives below 4G. we would have no choice but extend this range.

The latest revision of the patch initializes all vm_pages except for those that are initially added to the vm_phys allocator. Any pages sensitive to early startup will thus be initialized.

I am reluctant to initialize everything below 4GB: doing so eliminates the improvement for small VMs, and I cannot see any (more) concrete bugs that would be fixed as a result.

In D40403#920492, @kib wrote:

In D40403#920222, @markj wrote:

In D40403#920211, @kib wrote:

Suppose that a kernel dump happens very early, before pages are initialized. Then, e.g. kernel memory needs the backing pages to be functional for some variations of busdma to work. I know that at very least DMAR busdma requires it.

"kernel dump" meaning minidump? I think it is not a problem, since the minidump code always excludes free pages (they are not set in the vm_page_dump bitset), and non-free pages are always initialized.

I am not sure I follow. dump_add_page() is only needed for the page that are not mapped into the KVA (excluding DMAP). All active mappings except DMAP are explicitly dumped regardless of the state of the dump bitmap. How would the kernel data be dumped otherwise?

So I think a lot of pages are not initialized until later, but they are potentially dumpable.

Only free (in the sense of belonging to the vm_phys buddy allocator) pages are potentially uninitialized. But such pages would not be mapped into KVA except via the DMAP, so I do not see the problem.

@markj Is this waiting for anything else before it lands?

In D40403#930878, @cperciva wrote:

@markj Is this waiting for anything else before it lands?

I am not sure it's a good idea to land the patch right before stable/14 is created. I've done a fair bit of testing locally but there's lots of potential here for bugs/races which are specific to a certain platform. Since such bugs might take a while to flesh out, I'd prefer to wait until after the branch is created.

What follows may be a series of stupid questions. If so, feel free to ignore them.

Some of the work of vm_page_init_page is setting fields to non-zero constants. That those constants are non-zero is the result of choices elsewhere in the code - where it was decided that PQ_NONE would be 255, for example. To what extent would changes that made more of these constants zero speed up initialization? I'm told that you cannot expect the fields of the vm_page struct to be zero on the call to vm_page_init_page. If so, would bzeroing the whole struct before setting the nonzero fields help performance at all, especially if there were fewer nonzero fields? Or bzeroing all the page structs before any calls to vm_page_init_page? And then setting only the nonzero fields.

In D40403#931222, @markj wrote:

In D40403#930878, @cperciva wrote:

@markj Is this waiting for anything else before it lands?

I am not sure it's a good idea to land the patch right before stable/14 is created. I've done a fair bit of testing locally but there's lots of potential here for bugs/races which are specific to a certain platform. Since such bugs might take a while to flesh out, I'd prefer to wait until after the branch is created.

Makes sense. We released 14.0 a while ago now; is it time to land this?

In D40403#949058, @dougm wrote:

What follows may be a series of stupid questions. If so, feel free to ignore them.

Some of the work of vm_page_init_page is setting fields to non-zero constants. That those constants are non-zero is the result of choices elsewhere in the code - where it was decided that PQ_NONE would be 255, for example. To what extent would changes that made more of these constants zero speed up initialization? I'm told that you cannot expect the fields of the vm_page struct to be zero on the call to vm_page_init_page. If so, would bzeroing the whole struct before setting the nonzero fields help performance at all, especially if there were fewer nonzero fields? Or bzeroing all the page structs before any calls to vm_page_init_page? And then setting only the nonzero fields.

I'm sorry, I meant to reply to this. In fact, we at one point did zero the whole array before setting non-zero fields. This was changed in https://cgit.freebsd.org/src/commit/?id=f93f7cf199b6b07464760dbbbcae84edead6e1ee. At the time I experimented with a few different approaches, including bzeroing each structure before setting non-zero fields, but I couldn't find one that gave a significant benefit over the others.

@alc and @dougm, do you have any comments on the updated patch?

What happens if we increase VM_NFREEORDER, e.g., 262144, to support 1 GB allocations? I think that you might might want to have a different constant to cap the lazy init chunk size.

In D40403#994971, @alc wrote:

What happens if we increase VM_NFREEORDER, e.g., 262144, to support 1 GB allocations? I think that you might might want to have a different constant to cap the lazy init chunk size.

The intent being to limit the overhead of a memory allocation in the worst case? Yes, that seems like a reasonable thing to do.

As far as the implementation goes, my inclination is to change vm_phys_alloc_freelist_pages() and vm_phys_alloc_npages() to 1) initialize only, say, 16MB worth of pages, 2) return the excess pages back to the init free pool rather than to the target free pool.

In D40403#994971, @alc wrote:

What happens if we increase VM_NFREEORDER, e.g., 262144, to support 1 GB allocations? I think that you might might want to have a different constant to cap the lazy init chunk size.

Thinking about this a bit more: if we changed VM_NFREEORDER, any allocation which requires the chunk to be lazily initialized would otherwise have to visit each page in the chunk to set the pool field of each vm_page. Do we care about limiting that overhead as well? That is, should the "lazy init chunk size" really be a "pool init chunk size"?

@alc @dougm Do you have any more comments on this patch?

In D40403#1036869, @markj wrote:

@alc @dougm Do you have any more comments on this patch?

I'll take a look.

@dougm has recently developed a patch, D45409, that updates the pool fields lazily when a block of pages moves between pools. The lazy updates happen at the same time as other mutations to the order field, so the run-time cost is likely near zero.