Messages in this thread | | | Subject | Re: [PATCH] xen: issue warning message when out of grant maptrack entries | From | Boris Ostrovsky <> | Date | Wed, 19 Sep 2018 09:29:04 -0400 |
| |
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@suse.com> >>> --- >>> 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?
-boris
>> >> Should we pass 'i' instead of count to set_foreign_p2m_mapping() below? >> The loop there will skip entries that are in error, but does it make >> sense to do the hypercall for kmap_ops with count>i ? > The loop is running until the end, so i == count for the call of kmap_ops(). > > > Juergen
| |