Re: [PATCH 1/9] gnttab: defer allocation of maptrack frames table

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 26/08/2021 11:09, Jan Beulich wrote:
> By default all guests are permitted to have up to 1024 maptrack frames,
> which on 64-bit means an 8k frame table. Yet except for driver domains
> guests normally don't make use of grant mappings. Defer allocating the
> table until a map track handle is first requested.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I continue to be unconvinced that it is a good idea to allow all DomU-s
> 1024 maptrack frames by default. While I'm still of the opinion that a
> hypervisor enforced upper bound is okay, I question this upper bound
> also getting used as the default value - this is perhaps okay for Dom0,
> but not elsewhere.
I agree here. I think having a per-domain limit maptrack is a good idea.

> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -633,6 +633,34 @@ get_maptrack_handle(
>       if ( likely(handle != INVALID_MAPTRACK_HANDLE) )
>           return handle;
>   
> +    if ( unlikely(!read_atomic(&lgt->maptrack)) )
> +    {
> +        struct grant_mapping **maptrack = NULL;
> +
> +        if ( lgt->max_maptrack_frames )
> +            maptrack = vzalloc(lgt->max_maptrack_frames * sizeof(*maptrack));
While I understand that allocating with a lock is bad idea, I don't like 
the fact that multiple vCPUs racing each other would result to 
temporarily allocate more memory.
If moving the call within the lock is not a solution, would using a loop 
with a cmpxchg() a solution to block the other vCPU?
Cheers,

--
Julien Grall