Re: [Minios-devel] [UNIKRAFT PATCH] plat/linuxu: Allocate heap only once on ukplat_meregion_count()

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Simon,

This patch seems fine. A suggestion to the patch.

Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>

Thanks & Regards

Sharan

On 9/13/19 4:40 PM, Simon Kuenzer wrote:
> Allocates a heap region only once when ukplat_meregion_count() is
> called. This is fixing an issue that caused allocating a new heap
> region whenever ukplat_memregion_count() was called.
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
>   plat/linuxu/memory.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/plat/linuxu/memory.c b/plat/linuxu/memory.c
> index bdea2aae..9b5479e8 100644
> --- a/plat/linuxu/memory.c
> +++ b/plat/linuxu/memory.c
> @@ -53,7 +53,7 @@ static int __linuxu_plat_heap_init(void)
>         int rc = 0;
>   
>   	_liblinuxuplat_opts.heap.len = heap_size * MB2B;
> -       uk_pr_info("Heap size %u\n", heap_size);
> +       uk_pr_info("Allocate memory for heap (%u MiB)\n", heap_size);
>   
>   	/**
>          * Allocate heap memory
> @@ -74,10 +74,21 @@ static int __linuxu_plat_heap_init(void)
>   
>   int ukplat_memregion_count(void)
>   {
> +       static int have_heap = 0;
Instead of introducing the have_heap, maybe we set the 
`_liblinuxuplat_opts.heap.len` only on success and use it to 
differentiate the error case.
>         int rc = 0;
>   
> -	rc = __linuxu_plat_heap_init();
> -       return (rc == 0) ? 1 : 0;
> +       if (!have_heap) {
> +               /*
> +                * NOTE: The heap size can be changed by a library parameter.
> +                * We assume that those ones are processed by the boot library
> +                * shortly before memory regions are scanned. This is why
> +                * we initialize the heap here.
> +                */
> +               rc = __linuxu_plat_heap_init();
> +               have_heap = (rc == 0) ? 1 : 0;
> +       }
> +
> +       return (have_heap) ? 1 : 0;
>   }
>   
>   int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m)
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel