Re: [Xen-devel] [PATCH v6 5/5] domain: use PGC_extra domheap page for shared_info

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.03.2020 18:49, paul@xxxxxxx wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
> 
> Currently shared_info is a shared xenheap page but shared xenheap pages
> complicate future plans for live-update of Xen so it is desirable to,
> where possible, not use them [1]. This patch therefore converts shared_info
> into a PGC_extra domheap page. This does entail freeing shared_info during
> domain_relinquish_resources() rather than domain_destroy() so care is
> needed to avoid de-referencing a NULL shared_info pointer hence some
> extra checks of 'is_dying' are needed.

If there's going to be agreement to follow this route, the implementation,
with a really minor cosmetic adjustment - see below -, looks okay to me.
Nevertheless I continue to dislike the implication from the extra care
that's now needed. As I think I have said before, I'd like to have at
least one other REST maintainer's opinion here.

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -260,9 +260,12 @@ void dump_pageframe_info(struct domain *d)
>  
>      page_list_for_each ( page, &d->extra_page_list )
>      {
> -        printk("    ExtraPage %p: caf=%08lx, taf=%" PRtype_info "\n",
> +        const char *tag = mfn_eq(page_to_mfn(page), d->shared_info.mfn) ?
> +            "[SHARED INFO]" : "";

Please can this be " [SHARED INFO]" with ...

> +        printk("    ExtraPage %p: caf=%08lx, taf=%" PRtype_info " %s\n",

... the blank before the final %s dropped here, such that we won't
have a trailing blank in the output?

Jan

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