Re: [UNIKRAFT PATCH 2/2] lib/ukmmap: Add page-alignment support to mmap/munmap

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hi Marius,

I think your patch should change the behavior of this mmap 
implementation: As far as I know, given address (we currently do not 
support this) should be page aligned and also the size should be in 
multiple of pages. If not, the size gets automatically aligned up.
You can send a v2 of this fix independent of D language support.

Thanks,

Simon

On 21.07.20 15:38, Marius-Cristian Baciu wrote:
> The D runtime library is dependent on this property
> ---
>   lib/ukmmap/mmap.c | 29 +++++++++++++++++++++++++++--
>   1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ukmmap/mmap.c b/lib/ukmmap/mmap.c
> index 7cee8dc..3422411 100644
> --- a/lib/ukmmap/mmap.c
> +++ b/lib/ukmmap/mmap.c
> @@ -38,6 +38,13 @@
>   #include <uk/alloc.h>
>   #include <string.h>
>   
> +#ifdef DRUNTIME
> +#include <uk/essentials.h>
> +
> +#define size_to_num_pages(size) \
> +       (ALIGN_UP((unsigned long)(size), __PAGE_SIZE) / __PAGE_SIZE)
A minor thing: You could use __PAGE_SHIFT in order to avoid the div 
operation.
> +#endif
> +
>   struct mmap_addr {
>         void *begin;
>         void *end;
> @@ -94,8 +101,12 @@ void *mmap(void *addr, size_t len, int prot,
>                 last = tmp;
>                 tmp = tmp->next;
>         }
> -       void *mem = uk_malloc(uk_alloc_get_default(), len);
> -
> +#ifdef DRUNTIME
> +    int num_pages = size_to_num_pages(len);
> +       void *mem = uk_palloc(uk_alloc_get_default(), num_pages);
This should be the normal behavior, remove the else cases.

> +#else
> +    void *mem = uk_malloc(uk_alloc_get_default(), len);
> +#endif
>         if (!mem) {
>                 errno = ENOMEM;
>                 return (void *) -1;
> @@ -131,6 +142,7 @@ int munmap(void *addr, size_t len)
>         if (!addr)
>                 return 0;
>         while (tmp) {
> +#ifndef DRUNTIME
>                 if (addr != tmp->begin) {
>                         if (tmp->end > addr + len) {
>                                 errno = EINVAL;
> @@ -158,6 +170,19 @@ int munmap(void *addr, size_t len)
>                         uk_free(uk_alloc_get_default(), addr);
>                         return 0;
>                 }
> +#else
> +        if (addr == tmp->begin) {
> +                       int num_pages = size_to_num_pages(len);
> +
> +                       if (!prev)
> +                               mmap_addr = tmp->next;
> +                       else
> +                               prev->next = tmp->next;
> +                       uk_free(uk_alloc_get_default(), tmp);
> +                       uk_pfree(uk_alloc_get_default(), addr, num_pages);
> +                       break;
> +        }
> +#endif
>                 prev = tmp;
>                 tmp = tmp->next;
>         }