Re: vm_object shadow pages and task termination

[Michael Kelly <mike@weatherwax.co.uk>]

On 07/06/2025 21:47, Samuel Thibault wrote:
> I don't know about shadow objects more than what the code comments say,
> so can't easily say much about the fix, but it seems to make some sense
> (probably indeally we'd avoid going down the chain for the parts that is
> covered by the upper object, but better be safe than fast)

I see what you mean but I cannot see a simple way of achieving that 
optimisation. Any success here would apply equally to 
vm_object_pmap_protect().

> There is one thing, however: I see in some places going through shadows
> that they lock the next object before unlocking the previous. This
> indeed seems safer against object->shadow changing under our feet.
That is my mistake and you are correct. See the revised patch at the end 
of this message which addresses that. I've kept the code in the same 
style as the existing code of vm_object.c (ie. use of while(1)/break).
> Have you tried to exercice your box with the patch? (e.g. building
> packages etc.)

I have built gnumach and hurd packages with the revised patch. I'm 
letting gcc build in the background.

Mike.

diff --git a/vm/vm_object.c b/vm/vm_object.c
index 3e6b28c4..35221b60 100644
--- a/vm/vm_object.c
+++ b/vm/vm_object.c
@@ -986,12 +986,30 @@ void vm_object_pmap_remove(
                return;

        vm_object_lock(object);
-       queue_iterate(&object->memq, p, vm_page_t, listq) {
+
+       while (TRUE)
+         {
+           queue_iterate(&object->memq, p, vm_page_t, listq) {
                if (!p->fictitious &&
                    (start <= p->offset) &&
                    (p->offset < end))
                        pmap_page_protect(p->phys_addr, VM_PROT_NONE);
-       }
+           }
+
+           if (object->shadow != VM_OBJECT_NULL)
+             {
+               vm_object_t prev_object = object;
+
+               object = object->shadow;
+               start += object->shadow_offset;
+               end   += object->shadow_offset;
+               vm_object_lock(object);
+               vm_object_unlock(prev_object);
+             }
+           else
+             break;
+         }
+
        vm_object_unlock(object);
 }