Re: [Xen-devel] [PATCH 09/11] gnttab: avoid spurious maptrack handle allocation failures

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/06/17 10:37, Jan Beulich wrote:
> When no memory is available in the hypervisor, rather than immediately
> failing the request try to steal a handle from another vCPU.

"request, try"

>
> Reported-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -397,7 +397,7 @@ get_maptrack_handle(
>      struct vcpu          *curr = current;
>      unsigned int          i, head;
>      grant_handle_t        handle;
> -    struct grant_mapping *new_mt;
> +    struct grant_mapping *new_mt = NULL;
>  
>      handle = __get_maptrack_handle(lgt, curr);
>      if ( likely(handle != -1) )
> @@ -408,8 +408,13 @@ get_maptrack_handle(
>      /*
>       * If we've run out of frames, try stealing an entry from another
>       * VCPU (in case the guest isn't mapping across its VCPUs evenly).
> +     * Also use this path in case we're out of memory, to avoid spurious
> +     * failures.
>       */
> -    if ( nr_maptrack_frames(lgt) >= max_maptrack_frames )
> +    if ( nr_maptrack_frames(lgt) < max_maptrack_frames )
> +        new_mt = alloc_xenheap_page();
> +
> +    if ( !new_mt )
>      {
>          /*
>           * Can drop the lock since no other VCPU can be adding a new

* frame once they've run out.
*/

It doesn't look like this comment is true any more, which brings the
locking correctness into question.

~Andrew

> @@ -432,12 +437,6 @@ get_maptrack_handle(
>          return steal_maptrack_handle(lgt, curr);
>      }
>  
> -    new_mt = alloc_xenheap_page();
> -    if ( !new_mt )
> -    {
> -        spin_unlock(&lgt->maptrack_lock);
> -        return -1;
> -    }
>      clear_page(new_mt);
>  
>      /*
>
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel