Latest version after the rounds of bugfixes

In D45119#1029511, @markj wrote:

I do not see how this solves the problem that @pho found. There, vm_map_process_deferred() was releasing writecounts, and in one case the backing vnode was doomed, so OBJ_TMPFS_VREF was clear.

Reduce scope for object1 and color variables.
Assign OBJT_DEAD type to object before deallocating it on the path after failing ctr, since we do not want to have the device dtr called.

Use vm_object_deallocate() instead of _terminate() on cleanup path.
Defer OBJ_COLORED setting after ctr provides the color.

In D45058#1029152, @tijl wrote:

I spent yesterday and today on this but it's taking too much time to familiarise myself with the code so I won't be able to fix this in a timely manner. It's also too risky to make changes to drm-*-kmod this close to 14.1. So I've created D45125 which should be safe to MFC.

Are the following statements correct:

• Physical RAM may be divided in segments with address ranges that are separated from each other.

Right

Remove refcount.h

Remove invalid assert about zero writemappings of the object for the new vnode (since node might be old).
Add asserts that we do not underflow writemapping counters for swap pagers.

In D44306#1028427, @ashafer_badland.io wrote:

I believe that would work and I agree it would be much simpler. @kib is there something the (b) variant provides that (a) would be missing if we went with @jhb's proposal? It seems like they provide the same functionality?

Check for object == NULL

Use vm_object_deallocate()

Handle first batch of comments.

In D45058#1027959, @markj wrote:

Include missing headers sys/queue.h from vm/_vm_phys.h for TAILQ, and sys/param.h from vm/vm_phys.h for NULL so linux/io.h doesn't have to include them.

I think this part should be committed separately.

arch_io_reserve_memtype_wc is implemented using PHYS_TO_DMAP and pmap_change_attr but not all architectures have a DMAP and this doesn't match the upstream implementation.

Looking at Linux, the old version of this diff actually does partially match the implementation. If the platform doesn't have a direct map, or the PA range isn't backed by RAM, then it does nothing. Otherwise it updates the mapping type of the range in the direct map, presumably to avoid the UB that kib mentions.

I think what this function is supposed to do is check whether the PA range is covered by the vm_page_array and update the direct map if so, otherwise register a fictitious range. Perhaps this is best implemented in the VM system rather than the LinuxKPI?

In D45074#1027845, @dim wrote:

Does the cxa_thread_dtor::dso field serve any function after this change? I couldn't see any other references to it.

As I noted, I am not sure that vm_phys_fictitious_reg() KPI is prepared for such use. IMO it needs to be enhanced to check if the registering range is already backed by some vm_page_t storage and not create an overlap. Right now there are only minor piece of such care, for the case where the range intersects with vm_page_array[]. Otherwise the result is UB both for memattr use, and for consequent RBTREE manipulations.

Mentioning the supposed scope of use for flag in the review would be best. BTW if it is internal to vm allocators, why not call it M_VM_NOFREE?

u_long

Why not M_NOFREE?

Please upload patches with full context.

Can you upload the full context for the diff?

In D44271#1026288, @brooks wrote:

In D44271#1025583, @kib wrote:

Private versioning namespace should be filtered out, I think.

I think we want to know when that set changes even if we don't consider it part of the ABI, but I probably could be persuaded otherwise. I might want to make it optional so libraries can opt in and others opt out.

In D44910#1026299, @jhb wrote:

In D44910#1024125, @kib wrote:

Could the purpose of the new cpu method expressed as "move CPU state to PCB/thread save location in preparation for ..."? I think a comment like that would be due.

Where would you want that comment to go? Next to its prototype in <sys/proc.h>?

Grammar updates.