Re: [Xen-devel] [PATCH] x86/mm: silence a pointless warning

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.05.17 at 17:15, <JBeulich@xxxxxxxx> wrote:
> get_page() logs a message when it fails (dom_cow is never dying or
> paging_mode_external()), so better avoid the call when it's pointless
> to do anyway.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Possibly we could be even more rigid and bail right away if ->is_dying
> is set.
> 
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -501,9 +501,9 @@ struct page_info *get_page_from_gfn_p2m(
>                  if ( fdom == NULL )
>                      page = NULL;
>              }
> -            else if ( !get_page(page, d)
> +            else if ( !get_page(page, d) &&
>                        /* Page could be shared */
> -                      && !get_page(page, dom_cow) )
> +                      (!p2m_is_shared(*t) || !get_page(page, dom_cow)) )
>                  page = NULL;
>          }
>          p2m_read_unlock(p2m);

The downside of this change is that they will turn silent what may
be a hint towards a reason for one of the long standing migration
issues we have (these warnings have appeared in recent osstest
logs always in conjunction with a failed migration test). Locally I've
used

--- unstable.orig/xen/arch/x86/mm/p2m.c
+++ unstable/xen/arch/x86/mm/p2m.c
@@ -480,6 +480,12 @@ struct page_info *get_page_from_gfn_p2m(
     p2m_access_t _a;
     p2m_type_t _t;
     mfn_t mfn;
+static unsigned long cnt, thr;//temp
+if(d->is_dying && ++cnt > thr) {//temp
+ cnt |= thr;
+ printk("%pv: d%d dying (look up %lx)\n", current, d->domain_id, gfn);
+ dump_execution_state();
+}
 
     /* Allow t or a to be NULL */
     t = t ?: &_t;

but with about a dozen migrations I didn't get this to trigger. I
therefore wonder whether we shouldn't, for a while, have
something like this in master.

Jan


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