Re: [PATCH 2/3 gnumach] vm_map_lookup: Add parameter for exiting with map locked

[Samuel Thibault]

(sorry my mail server kept the first mails for itself for some time...)

Damien Zammit, le mer. 21 févr. 2024 13:46:05 +0000, a ecrit:
> diff --git a/vm/vm_map.c b/vm/vm_map.c
> index e4672260..090a5c9c 100644
> --- a/vm/vm_map.c
> +++ b/vm/vm_map.c
> @@ -4627,7 +4627,8 @@ vm_map_t vm_map_fork(vm_map_t old_map)
>   *   is returned.
>   *
>   *   The map should not be locked; it will not be
> - *   locked on exit.  In order to guarantee the
> + *   locked on exit unless exit_map_locked is set.
> + *   In order to guarantee the
>   *   existence of the returned object, it is returned
>   *   locked.
>   *

As I mentioned in my other response, I'd say that on failure we should
release the map as well, that looks more in line with usual practices
and keeping the map locked will probably not be useful to anybody.

Concerning the naming, I'd rather call it keep_map_locked.

> @@ -4640,6 +4641,7 @@ kern_return_t vm_map_lookup(
>       vm_map_t                *var_map,       /* IN/OUT */
>       vm_offset_t             vaddr,
>       vm_prot_t               fault_type,
> +     boolean_t               exit_map_locked,
>  
>       vm_map_version_t        *out_version,   /* OUT */
>       vm_object_t             *object,        /* OUT */
> @@ -4661,7 +4663,8 @@ kern_return_t vm_map_lookup(
>  
>  #define      RETURN(why) \
>               { \
> -             vm_map_unlock_read(map); \
> +             if (!exit_map_locked) \

So it'd mean an additional && why == KERN_SUCCESS here.

> +               vm_map_unlock_read(map); \
>               return(why); \
>               }
>  

Samuel