Applied, thanks!
Sergey Bugaev, le mer. 05 juil. 2023 17:16:37 +0300, a ecrit:
> vm_object_coalesce() callers used to rely on the fact that it always
> merged the next_object into prev_object, potentially destroying
> next_object and leaving prev_object the result of the whole operation.
>
> After ee65849bec5da261be90f565bee096abb4117bdd
> "vm: Allow coalescing null object with an internal object", this is no
> longer true, since in case of prev_object == VM_OBJECT_NULL and
> next_object != VM_OBJECT_NULL, the overall result is next_object, not
> prev_object. The current callers are prepared to deal with this since
> they handle this case seprately anyway, but the following commit will
> introduce another caller that handles both cases in the same code path.
>
> So, declare the way vm_object_coalesce() coalesces the two objects its
> implementation detail, and make it return the resulting object and the
> offset into it explicitly. This simplifies the callers, too.
> ---
> vm/vm_map.c | 11 ++++++-----
> vm/vm_object.c | 53 +++++++++++++++++++++++++++++++++++++-------------
> vm/vm_object.h | 4 +++-
> 3 files changed, 49 insertions(+), 19 deletions(-)
>
> diff --git a/vm/vm_map.c b/vm/vm_map.c
> index 97fc09ce..d147d4b2 100644
> --- a/vm/vm_map.c
> +++ b/vm/vm_map.c
> @@ -1089,7 +1089,9 @@ kern_return_t vm_map_enter(
> entry->offset,
> offset,
> (vm_size_t)(entry->vme_end - entry->vme_start),
> - size)) {
> + size,
> + &entry->object.vm_object,
> + &entry->offset)) {
>
> /*
> * Coalesced the two objects - can extend
> @@ -1099,7 +1101,6 @@ kern_return_t vm_map_enter(
> map->size += size;
> entry->vme_end = end;
> vm_map_gap_update(&map->hdr, entry);
> - vm_object_deallocate(object);
> RETURN(KERN_SUCCESS);
> }
> }
> @@ -1117,7 +1118,9 @@ kern_return_t vm_map_enter(
> offset,
> next_entry->offset,
> size,
> - (vm_size_t)(next_entry->vme_end -
> next_entry->vme_start))) {
> + (vm_size_t)(next_entry->vme_end -
> next_entry->vme_start),
> + &next_entry->object.vm_object,
> + &next_entry->offset)) {
>
> /*
> * Coalesced the two objects - can extend
> @@ -1126,9 +1129,7 @@ kern_return_t vm_map_enter(
> */
> map->size += size;
> next_entry->vme_start = start;
> - next_entry->offset -= size;
> vm_map_gap_update(&map->hdr, entry);
> - vm_object_deallocate(object);
> RETURN(KERN_SUCCESS);
> }
> }
> diff --git a/vm/vm_object.c b/vm/vm_object.c
> index e01c1856..c238cce4 100644
> --- a/vm/vm_object.c
> +++ b/vm/vm_object.c
> @@ -2687,15 +2687,16 @@ void vm_object_page_remove(
>
> /*
> * Routine: vm_object_coalesce
> - * Function: Coalesces two objects backing up adjoining
> - * regions of memory into a single object.
> - *
> - * returns TRUE if objects were combined.
> - *
> - * NOTE: Only works at the moment if one of the objects is NULL
> - * or if the objects are the same - otherwise, which
> - * object do we lock first?
> - *
> + * Purpose:
> + * Tries to coalesce two objects backing up adjoining
> + * regions of memory into a single object.
> + *
> + * NOTE: Only works at the moment if one of the objects
> + * is NULL or if the objects are the same - otherwise,
> + * which object do we lock first?
> + * Returns:
> + * TRUE if objects have been coalesced.
> + * FALSE the objects could not be coalesced.
> * Parameters:
> * prev_object First object to coalesce
> * prev_offset Offset into prev_object
> @@ -2705,8 +2706,14 @@ void vm_object_page_remove(
> * prev_size Size of reference to prev_object
> * next_size Size of reference to next_object
> *
> + * new_object Resulting colesced object
> + * new_offset Offset into the resulting object
> * Conditions:
> - * The object must *not* be locked.
> + * The objects must *not* be locked.
> + *
> + * If the objects are coalesced successfully, the caller's
> + * references for both objects are consumed, and the caller
> + * gains a reference for the new object.
> */
>
> boolean_t vm_object_coalesce(
> @@ -2715,7 +2722,9 @@ boolean_t vm_object_coalesce(
> vm_offset_t prev_offset,
> vm_offset_t next_offset,
> vm_size_t prev_size,
> - vm_size_t next_size)
> + vm_size_t next_size,
> + vm_object_t *new_object, /* OUT */
> + vm_offset_t *new_offset) /* OUT */
> {
> vm_object_t object;
> vm_size_t newsize;
> @@ -2725,10 +2734,23 @@ boolean_t vm_object_coalesce(
> * If neither object actually exists,
> * the offsets don't matter.
> */
> - if (prev_object == VM_OBJECT_NULL)
> + if (prev_object == VM_OBJECT_NULL) {
> + *new_object = VM_OBJECT_NULL;
> + *new_offset = 0;
> return TRUE;
> + }
>
> - return prev_offset + prev_size == next_offset;
> + if (prev_offset + prev_size == next_offset) {
> + *new_object = prev_object;
> + *new_offset = prev_offset;
> + /*
> + * Deallocate one of the two references.
> + */
> + vm_object_deallocate(prev_object);
> + return TRUE;
> + }
> +
> + return FALSE;
> }
>
> if (next_object != VM_OBJECT_NULL) {
> @@ -2785,6 +2807,8 @@ boolean_t vm_object_coalesce(
> newsize = prev_offset + prev_size + next_size;
> if (newsize > object->size)
> object->size = newsize;
> +
> + *new_offset = prev_offset;
> } else {
> /*
> * Check if we have enough space in the object
> @@ -2802,9 +2826,12 @@ boolean_t vm_object_coalesce(
> vm_object_page_remove(object,
> next_offset - prev_size,
> next_offset);
> +
> + *new_offset = next_offset - prev_size;
> }
>
> vm_object_unlock(object);
> + *new_object = object;
> return TRUE;
> }
>
> diff --git a/vm/vm_object.h b/vm/vm_object.h
> index 46328a38..9c17541f 100644
> --- a/vm/vm_object.h
> +++ b/vm/vm_object.h
> @@ -247,7 +247,9 @@ extern boolean_t vm_object_coalesce(
> vm_offset_t prev_offset,
> vm_offset_t next_offset,
> vm_size_t prev_size,
> - vm_size_t next_size);
> + vm_size_t next_size,
> + vm_object_t *new_object, /* OUT */
> + vm_offset_t *new_offset); /* OUT */
>
> extern void vm_object_pager_wakeup(ipc_port_t pager);
>
> --
> 2.41.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
