Re: [Xen-devel] [PATCH v5 3/6] x86 / pv: do not treat PGC_extra pages as RAM

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 10 March 2020 15:33
> To: paul@xxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Paul Durrant' <pdurrant@xxxxxxxxxx>; 
> 'Andrew Cooper'
> <andrew.cooper3@xxxxxxxxxx>; 'Wei Liu' <wl@xxxxxxx>; 'Roger Pau Monné' 
> <roger.pau@xxxxxxxxxx>
> Subject: Re: [PATCH v5 3/6] x86 / pv: do not treat PGC_extra pages as RAM
> 
> On 10.03.2020 16:16, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: 10 March 2020 15:13
> >> To: paul@xxxxxxx
> >> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Paul Durrant' <pdurrant@xxxxxxxxxx>; 
> >> 'Andrew Cooper'
> >> <andrew.cooper3@xxxxxxxxxx>; 'Wei Liu' <wl@xxxxxxx>; 'Roger Pau Monné' 
> >> <roger.pau@xxxxxxxxxx>
> >> Subject: Re: [PATCH v5 3/6] x86 / pv: do not treat PGC_extra pages as RAM
> >>
> >> On 10.03.2020 16:05, Paul Durrant wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@xxxxxxxx>
> >>>> Sent: 10 March 2020 14:59
> >>>>
> >>>> In reply to patch 6 I did suggest to
> >>>> have a separate list, but without taking these pages off
> >>>> d->page_list,
> >>>
> >>> How would that work without adding an extra page_list_entry into struct 
> >>> page_info?
> >>
> >> As said there, it'd be a singly linked list using a __pdx_t
> >> field just like there already is with "next_shadow", i.e.
> >> you'd add another union member "next_extra" or some such. Of
> >> course the list shouldn't grow too long, or else insertion
> >> and removal may become a bottleneck. Not sure how well this
> >> would fit Arm, though; maybe they wouldn't need this, but
> >> that depends on whether the list would be used for purposes
> >> beyond dumping.
> >
> > That seems more obscure and bug-prone than an extra list head
> > in struct domain. I'd really prefer to stick with that even
> > if it does make things a little more ugly until xenpage_list
> > goes away.
> 
> Okay with me if there really was no property (other than
> assign_pages() then needing to pick the right list, which is
> not much different from needing to put the extra pages on two
> lists) that you'd lose by no longer having the pages on the
> same list.

Just assign_pages() and arch_free_heap_page(), and my test patch defines:

+static inline struct page_list_head *page_to_list(
+    struct domain *d, const struct page_info *pg)
+{
+    if ( is_xen_heap_page(pg) )
+        return &d->xenpage_list;
+    else if ( pg->count_info & PGC_extra )
+        return &d->extra_page_list;
+
+    return &d->page_list;
+}

which they both use to select the right list.

Once xenpage_list goes away then this can be simplified to use a ternary 
operator.

  Paul

> 
> Jan


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