[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: vm_object shadow pages and task termination



Hello,

Michael Kelly, le sam. 07 juin 2025 21:16:13 +0100, a ecrit:
> So in essence, I added the same shadow chain recursion functionality to
> vm_object_pmap_remove(). This works successfully within my test case. I am
> however not experienced enough with the mach code to claim this as a certain
> fix but offer it here for consideration.

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)

> In particular I found it hard to get my head around the offset
> manipulations for the shadow chain. My patch includes that aspect but
> would need particular checking.

It seems good to me since p->offset is the offset inside the object.

> diff --git a/vm/vm_object.c b/vm/vm_object.c
> index 3e6b28c4..efb169da 100644
> --- a/vm/vm_object.c
> +++ b/vm/vm_object.c
> @@ -980,10 +980,11 @@ void vm_object_pmap_remove(
>         vm_offset_t     start,
>         vm_offset_t     end)
>  {
> -       vm_page_t       p;
> +  vm_page_t    p;
> 
> -       if (object == VM_OBJECT_NULL)
> -               return;
> +  while (object != VM_OBJECT_NULL)
> +    {
> +        vm_object_t next_object;
> 
>         vm_object_lock(object);

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.

Have you tried to exercice your box with the patch? (e.g. building
packages etc.)

Samuel

>         queue_iterate(&object->memq, p, vm_page_t, listq) {
> 
> @@ -992,7 +993,12 @@ void vm_object_pmap_remove(
>                     (p->offset < end))
>                         pmap_page_protect(p->phys_addr, VM_PROT_NONE);
>         }
> +       next_object = object->shadow;
> +       start += object->shadow_offset;
> +       end   += object->shadow_offset;
>         vm_object_unlock(object);
> +       object = next_object;
> +    }
>  }
> 
>  /*


Reply to: