Re: [Xen-devel] [PATCH] xen: issue warning message when out of grant maptrack entries

[Juergen Gross <jgross@xxxxxxxx>]

On 19/09/18 15:29, Boris Ostrovsky wrote:
> On 9/18/18 1:17 PM, Juergen Gross wrote:
>> On 18/09/18 19:03, Boris Ostrovsky wrote:
>>> On 9/18/18 5:32 AM, Juergen Gross wrote:
>>>> When a driver domain (e.g. dom0) is running out of maptrack entries it
>>>> can't map any more foreign domain pages. Instead of silently stalling
>>>> the affected domUs issue a rate limited warning in this case in order
>>>> to make it easier to detect that situation.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>>>> ---
>>>>  drivers/xen/grant-table.c | 25 +++++++++++++++++++------
>>>>  1 file changed, 19 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
>>>> index 7bafa703a992..09f6ff8c1957 100644
>>>> --- a/drivers/xen/grant-table.c
>>>> +++ b/drivers/xen/grant-table.c
>>>> @@ -1040,18 +1040,31 @@ int gnttab_map_refs(struct gnttab_map_grant_ref 
>>>> *map_ops,
>>>>            return ret;
>>>>  
>>>>    for (i = 0; i < count; i++) {
>>>> -          /* Retry eagain maps */
>>>> -          if (map_ops[i].status == GNTST_eagain)
>>>> -                  gnttab_retry_eagain_gop(GNTTABOP_map_grant_ref, map_ops 
>>>> + i,
>>>> -                                          &map_ops[i].status, __func__);
>>>> -
>>>> -          if (map_ops[i].status == GNTST_okay) {
>>>> +          switch (map_ops[i].status) {
>>>> +          case GNTST_okay:
>>>> +          {
>>>>                    struct xen_page_foreign *foreign;
>>>>  
>>>>                    SetPageForeign(pages[i]);
>>>>                    foreign = xen_page_foreign(pages[i]);
>>>>                    foreign->domid = map_ops[i].dom;
>>>>                    foreign->gref = map_ops[i].ref;
>>>> +                  break;
>>>> +          }
>>>> +
>>>> +          case GNTST_no_device_space:
>>>> +                  pr_warn_ratelimited("maptrack limit reached, can't map 
>>>> all guest pages\n");
>>>> +                  break;
>>>> +
>>>> +          case GNTST_eagain:
>>>> +                  /* Retry eagain maps */
>>>> +                  gnttab_retry_eagain_gop(GNTTABOP_map_grant_ref,
>>>> +                                          map_ops + i,
>>>> +                                          &map_ops[i].status, __func__);
>>>> +                  break;
>>>> +
>>>> +          default:
>>>> +                  break;
>>>>            }
>>>>    }
> 
> 
> After having a second look at this (and at the risk of embarrassing
> myself again with this patch) --- aren't we supposed to test status
> after gnttab_retry_eagain_gop() call, and then actually set foreign
> properties?

Right. Thanks for catching that!

V2 coming soon...


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel