bug-hurd
[Top][All Lists]
Advanced

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

Re: [PATCH gnumach v2] Implement per task virtual memory limit


From: Samuel Thibault
Subject: Re: [PATCH gnumach v2] Implement per task virtual memory limit
Date: Mon, 30 Dec 2024 19:08:55 +0100

Hello,

Please also add printing size_null in vm_map_print.

dnietoc@gmail.com, le mer. 25 déc. 2024 16:16:06 -0300, a ecrit:
> @@ -1160,6 +1226,8 @@ kern_return_t vm_map_enter(
>  
>       vm_map_entry_link(map, entry, new_entry);
>       map->size += size;
> +     if (max_protection == VM_PROT_NONE)
> +             map->size_none += size;
>  
>       /*
>        *      Update the free space hint and the lookup hint

In vm_map_enter, there are two other incrementations of map->size above,
in the coalescing cases: if the coalescing happens with a max_prot =
NONE entry we need to increase size_none two.

> @@ -1679,11 +1747,13 @@ kern_return_t vm_map_protect(
>               vm_map_clip_end(map, current, end);
>  
>               old_prot = current->protection;
> -             if (set_max)
> +             if (set_max) {
> +                     if (current->max_protection != new_prot && new_prot == 
> VM_PROT_NONE)
> +                             map->size_none += current->vme_end - 
> current->vme_start;

You also need to take into account the case where
current->max_protection was NONE and new_prot is not NONE, you then have
to decrease size_none.

>                       current->protection =
>                               (current->max_protection = new_prot) &
>                                       old_prot;
> -             else
> +             } else
>                       current->protection = new_prot;
>  
>               /*


It seems there is also an occurrence of changing max_protection in
projected_buffer_allocate and projected_buffer_map


> @@ -3055,6 +3136,11 @@ kern_return_t vm_map_copyout_page_list(
>  
>       vm_map_lock(dst_map);
>  
> +     if ((result = vm_map_enforce_limit(dst_map, size, 
> "vm_map_copyout_page_lists")) != KERN_SUCCESS) {
> +             vm_map_unlock(dst_map);
> +             return result;
> +     }
> +

For coherency with others, put it after checking against last == NULL
below?

>       last = vm_map_find_entry_anywhere(dst_map, size, 0, TRUE, &start);
>  
>       if (last == NULL) {


Please also add a note about not updating size_null just like you did
for vm_map_copyout.

> @@ -582,4 +587,12 @@ void _vm_map_clip_end(
>       vm_offset_t             end,
>       boolean_t               link_gap);
>  
> +/*
> + *      This function is called to inherit the virtual memory limits
> + *      from one vm_map_t to another.
> + */
> +void vm_map_copy_limits(
> +     vm_map_t src,
> +     vm_map_t dst);

I'd say rather exchange src and dst, to have it ordered like in memcpy.

Samuel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]