| Message ID | 20200130145745.1306-3-pdurrant@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | purge free_shared_domheap_page() | expand |
On 30.01.2020 15:57, Paul Durrant wrote: > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d) > > printk("Memory pages belonging to domain %u:\n", d->domain_id); > > - if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead ) > + if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead ) Before I go any further - are you simply replacing _all_ ->tot_pages uses by the new helper? In the case here, for example, I don't think this is what we want. Jan
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 30 January 2020 16:29 > To: Durrant, Paul <pdurrant@amazon.co.uk> > Cc: xen-devel@lists.xenproject.org; Andrew Cooper > <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné > <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian > Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Konrad > Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini > <sstabellini@kernel.org>; Tim Deegan <tim@xen.org> > Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function > > On 30.01.2020 15:57, Paul Durrant wrote: > > --- a/xen/arch/x86/domain.c > > +++ b/xen/arch/x86/domain.c > > @@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d) > > > > printk("Memory pages belonging to domain %u:\n", d->domain_id); > > > > - if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead ) > > + if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead ) > > Before I go any further - are you simply replacing _all_ > ->tot_pages uses by the new helper? Basically, apart from domain_adjust_tot_pages(), yes. > In the case here, for > example, I don't think this is what we want. > Why not? I would have thought any 'extra' pages would always be of interest. Paul
On 30.01.2020 17:32, Durrant, Paul wrote: >> -----Original Message----- >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 30 January 2020 16:29 >> To: Durrant, Paul <pdurrant@amazon.co.uk> >> Cc: xen-devel@lists.xenproject.org; Andrew Cooper >> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné >> <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian >> Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Konrad >> Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini >> <sstabellini@kernel.org>; Tim Deegan <tim@xen.org> >> Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function >> >> On 30.01.2020 15:57, Paul Durrant wrote: >>> --- a/xen/arch/x86/domain.c >>> +++ b/xen/arch/x86/domain.c >>> @@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d) >>> >>> printk("Memory pages belonging to domain %u:\n", d->domain_id); >>> >>> - if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead ) >>> + if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead ) >> >> Before I go any further - are you simply replacing _all_ >> ->tot_pages uses by the new helper? > > Basically, apart from domain_adjust_tot_pages(), yes. > >> In the case here, for >> example, I don't think this is what we want. >> > > Why not? I would have thought any 'extra' pages would always be of interest. Could be hundreds or thousands in the future. As long as it's in reality just one, perhaps it indeed doesn't matter much. I'll take a closer look tomorrow, to see if there are other places where the adjusted count would better not be used. From my partial audit in the morning I seem to recall that there were both kinds of situations... Jan
On 30.01.2020 15:57, Paul Durrant wrote: > v8: > - New in v8 > --- > xen/arch/x86/domain.c | 2 +- > xen/arch/x86/mm.c | 6 +++--- > xen/arch/x86/mm/p2m-pod.c | 10 +++++----- > xen/arch/x86/mm/shadow/common.c | 2 +- > xen/arch/x86/msi.c | 2 +- > xen/arch/x86/numa.c | 2 +- > xen/arch/x86/pv/dom0_build.c | 25 +++++++++++++------------ > xen/arch/x86/pv/domain.c | 2 +- > xen/common/domctl.c | 2 +- > xen/common/grant_table.c | 4 ++-- > xen/common/keyhandler.c | 2 +- > xen/common/memory.c | 4 ++-- > xen/common/page_alloc.c | 15 ++++++++------- > xen/include/public/memory.h | 4 ++-- > xen/include/xen/sched.h | 24 ++++++++++++++++++------ > 15 files changed, 60 insertions(+), 46 deletions(-) From this, with the comment you add next to the struct field, and with your reply yesterday, what about the uses in - arch/arm/arm64/domctl.c:switch_mode(), - arch/x86/pv/shim.c:pv_shim_setup_dom(), - arch/x86/pv/shim.c:write_start_info()? > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -4194,8 +4194,8 @@ long do_mmu_update( > * - page caching attributes cleaned up > * - removed from the domain's page_list > * > - * If MEMF_no_refcount is not set, the domain's tot_pages will be > - * adjusted. If this results in the page count falling to 0, > + * If MEMF_no_refcount is not set, the domain_adjust_tot_pages() will > + * be called. If this results in the page count falling to 0, > * put_domain() will be called. If you fiddle with this comment, please also drop the "the" ahead of the function name. Unless you as a native speaker would confirm it's appropriate there (it doesn't seem so to me). Of course I also wouldn't mind leaving this untouched altogether. > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -717,7 +717,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) > > /* > * Pages in in_chunk_list is stolen without > - * decreasing the tot_pages. If the domain is dying when > + * decreasing domain_tot_pages(). If the domain is dying when I'd leave this comment alone, or at least not use the function name. Maybe do as you did in the public header? > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -364,12 +364,18 @@ struct domain > spinlock_t page_alloc_lock; /* protects all the following fields */ > struct page_list_head page_list; /* linked list */ > struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */ > - unsigned int tot_pages; /* number of pages currently possesed */ > - unsigned int xenheap_pages; /* # pages allocated from Xen heap */ > - unsigned int outstanding_pages; /* pages claimed but not possessed */ > - unsigned int max_pages; /* maximum value for tot_pages */ > - atomic_t shr_pages; /* number of shared pages */ > - atomic_t paged_pages; /* number of paged-out pages */ > + > + /* > + * This field should only be directly accessed by domain_adjust_tot_pages() > + * and the domain_tot_pages() helper function defined below. > + */ > + unsigned int tot_pages; If the three missing ones got taken care of, with there being arguments both pro and con your change to dump_pageframe_info(), I'd be okay with it getting changed as you do, to not render this comment partially wrong. Jan
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 31 January 2020 12:53 > To: Durrant, Paul <pdurrant@amazon.co.uk> > Cc: xen-devel@lists.xenproject.org; Andrew Cooper > <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné > <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian > Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Konrad > Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini > <sstabellini@kernel.org>; Tim Deegan <tim@xen.org> > Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function > > On 30.01.2020 15:57, Paul Durrant wrote: > > v8: > > - New in v8 > > --- > > xen/arch/x86/domain.c | 2 +- > > xen/arch/x86/mm.c | 6 +++--- > > xen/arch/x86/mm/p2m-pod.c | 10 +++++----- > > xen/arch/x86/mm/shadow/common.c | 2 +- > > xen/arch/x86/msi.c | 2 +- > > xen/arch/x86/numa.c | 2 +- > > xen/arch/x86/pv/dom0_build.c | 25 +++++++++++++------------ > > xen/arch/x86/pv/domain.c | 2 +- > > xen/common/domctl.c | 2 +- > > xen/common/grant_table.c | 4 ++-- > > xen/common/keyhandler.c | 2 +- > > xen/common/memory.c | 4 ++-- > > xen/common/page_alloc.c | 15 ++++++++------- > > xen/include/public/memory.h | 4 ++-- > > xen/include/xen/sched.h | 24 ++++++++++++++++++------ > > 15 files changed, 60 insertions(+), 46 deletions(-) > > From this, with the comment you add next to the struct field, and > with your reply yesterday, what about the uses in > - arch/arm/arm64/domctl.c:switch_mode(), TBH I'm not sure with that one. It looks to me like it needs to check whether the domain has *any* memory assigned. Perhaps checking page_list would be more appropriate. Perhaps Julien can comment? > - arch/x86/pv/shim.c:pv_shim_setup_dom(), > - arch/x86/pv/shim.c:write_start_info()? It looks like both of those should be changed. > > > --- a/xen/arch/x86/mm.c > > +++ b/xen/arch/x86/mm.c > > @@ -4194,8 +4194,8 @@ long do_mmu_update( > > * - page caching attributes cleaned up > > * - removed from the domain's page_list > > * > > - * If MEMF_no_refcount is not set, the domain's tot_pages will be > > - * adjusted. If this results in the page count falling to 0, > > + * If MEMF_no_refcount is not set, the domain_adjust_tot_pages() will > > + * be called. If this results in the page count falling to 0, > > * put_domain() will be called. > > If you fiddle with this comment, please also drop the "the" ahead > of the function name. Unless you as a native speaker would confirm > it's appropriate there (it doesn't seem so to me). Of course I > also wouldn't mind leaving this untouched altogether. > Ok, I'll drop that hunk. > > --- a/xen/common/memory.c > > +++ b/xen/common/memory.c > > @@ -717,7 +717,7 @@ static long > memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) > > > > /* > > * Pages in in_chunk_list is stolen without > > - * decreasing the tot_pages. If the domain is dying > when > > + * decreasing domain_tot_pages(). If the domain is > dying when > > I'd leave this comment alone, or at least not use the function > name. Maybe do as you did in the public header? > OK I'll leave this alone too. > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -364,12 +364,18 @@ struct domain > > spinlock_t page_alloc_lock; /* protects all the following > fields */ > > struct page_list_head page_list; /* linked list */ > > struct page_list_head xenpage_list; /* linked list (size > xenheap_pages) */ > > - unsigned int tot_pages; /* number of pages currently > possesed */ > > - unsigned int xenheap_pages; /* # pages allocated from Xen > heap */ > > - unsigned int outstanding_pages; /* pages claimed but not > possessed */ > > - unsigned int max_pages; /* maximum value for tot_pages > */ > > - atomic_t shr_pages; /* number of shared pages > */ > > - atomic_t paged_pages; /* number of paged-out pages > */ > > + > > + /* > > + * This field should only be directly accessed by > domain_adjust_tot_pages() > > + * and the domain_tot_pages() helper function defined below. > > + */ > > + unsigned int tot_pages; > > If the three missing ones got taken care of, with there being arguments > both pro and con your change to dump_pageframe_info(), I'd be okay with > it getting changed as you do, to not render this comment partially > wrong. Ok. Paul > > Jan
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Durrant, Paul > Sent: 31 January 2020 15:19 > To: Jan Beulich <jbeulich@suse.com>; Julien Grall <julien@xen.org> > Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap > <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>; > Ian Jackson <ian.jackson@eu.citrix.com>; Tim Deegan <tim@xen.org>; xen- > devel@lists.xenproject.org; Roger Pau Monné <roger.pau@citrix.com> > Subject: Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper > function > > > -----Original Message----- > > From: Jan Beulich <jbeulich@suse.com> > > Sent: 31 January 2020 12:53 > > To: Durrant, Paul <pdurrant@amazon.co.uk> > > Cc: xen-devel@lists.xenproject.org; Andrew Cooper > > <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné > > <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian > > Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; > Konrad > > Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini > > <sstabellini@kernel.org>; Tim Deegan <tim@xen.org> > > Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function > > > > On 30.01.2020 15:57, Paul Durrant wrote: > > > v8: > > > - New in v8 > > > --- > > > xen/arch/x86/domain.c | 2 +- > > > xen/arch/x86/mm.c | 6 +++--- > > > xen/arch/x86/mm/p2m-pod.c | 10 +++++----- > > > xen/arch/x86/mm/shadow/common.c | 2 +- > > > xen/arch/x86/msi.c | 2 +- > > > xen/arch/x86/numa.c | 2 +- > > > xen/arch/x86/pv/dom0_build.c | 25 +++++++++++++------------ > > > xen/arch/x86/pv/domain.c | 2 +- > > > xen/common/domctl.c | 2 +- > > > xen/common/grant_table.c | 4 ++-- > > > xen/common/keyhandler.c | 2 +- > > > xen/common/memory.c | 4 ++-- > > > xen/common/page_alloc.c | 15 ++++++++------- > > > xen/include/public/memory.h | 4 ++-- > > > xen/include/xen/sched.h | 24 ++++++++++++++++++------ > > > 15 files changed, 60 insertions(+), 46 deletions(-) > > > > From this, with the comment you add next to the struct field, and > > with your reply yesterday, what about the uses in > > - arch/arm/arm64/domctl.c:switch_mode(), > > TBH I'm not sure with that one. It looks to me like it needs to check > whether the domain has *any* memory assigned. Perhaps checking page_list > would be more appropriate. Perhaps Julien can comment? > Having chatted to Julien, the aim of checking tot_pages here is to test whether the domain is newly created, in which case using domain_tot_pages() is the appropriate thing to do. Paul
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 28fefa1f81..643c23ffb0 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d) printk("Memory pages belonging to domain %u:\n", d->domain_id); - if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead ) + if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead ) { printk(" DomPage list too long to display\n"); } diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index f50c065af3..8bb66cf30c 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4194,8 +4194,8 @@ long do_mmu_update( * - page caching attributes cleaned up * - removed from the domain's page_list * - * If MEMF_no_refcount is not set, the domain's tot_pages will be - * adjusted. If this results in the page count falling to 0, + * If MEMF_no_refcount is not set, the domain_adjust_tot_pages() will + * be called. If this results in the page count falling to 0, * put_domain() will be called. * * The caller should either call free_domheap_page() to free the @@ -4870,7 +4870,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) else if ( rc >= 0 ) { p2m = p2m_get_hostp2m(d); - target.tot_pages = d->tot_pages; + target.tot_pages = domain_tot_pages(d); target.pod_cache_pages = p2m->pod.count; target.pod_entries = p2m->pod.entry_count; diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c index 096e2773fb..f2c9409568 100644 --- a/xen/arch/x86/mm/p2m-pod.c +++ b/xen/arch/x86/mm/p2m-pod.c @@ -302,7 +302,7 @@ out: * The following equations should hold: * 0 <= P <= T <= B <= M * d->arch.p2m->pod.entry_count == B - P - * d->tot_pages == P + d->arch.p2m->pod.count + * domain_tot_pages(d) == P + d->arch.p2m->pod.count * * Now we have the following potential cases to cover: * B <T': Set the PoD cache size equal to the number of outstanding PoD @@ -336,7 +336,7 @@ p2m_pod_set_mem_target(struct domain *d, unsigned long target) pod_lock(p2m); /* P == B: Nothing to do (unless the guest is being created). */ - populated = d->tot_pages - p2m->pod.count; + populated = domain_tot_pages(d) - p2m->pod.count; if ( populated > 0 && p2m->pod.entry_count == 0 ) goto out; @@ -348,7 +348,7 @@ p2m_pod_set_mem_target(struct domain *d, unsigned long target) * T' < B: Don't reduce the cache size; let the balloon driver * take care of it. */ - if ( target < d->tot_pages ) + if ( target < domain_tot_pages(d) ) goto out; pod_target = target - populated; @@ -1231,8 +1231,8 @@ out_of_memory: pod_unlock(p2m); printk("%s: Dom%d out of PoD memory! (tot=%"PRIu32" ents=%ld dom%d)\n", - __func__, d->domain_id, d->tot_pages, p2m->pod.entry_count, - current->domain->domain_id); + __func__, d->domain_id, domain_tot_pages(d), + p2m->pod.entry_count, current->domain->domain_id); domain_crash(d); return false; out_fail: diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c index 6212ec2c4a..cba3ab1eba 100644 --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -1256,7 +1256,7 @@ static unsigned int sh_min_allocation(const struct domain *d) * up of slot zero and an LAPIC page), plus one for HVM's 1-to-1 pagetable. */ return shadow_min_acceptable_pages(d) + - max(max(d->tot_pages / 256, + max(max(domain_tot_pages(d) / 256, is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + 2 : 0U) + is_hvm_domain(d), d->arch.paging.shadow.p2m_pages); diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c index df97ce0c72..2fabaaa155 100644 --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -991,7 +991,7 @@ static int msix_capability_init(struct pci_dev *dev, seg, bus, slot, func, d->domain_id); if ( !is_hardware_domain(d) && /* Assume a domain without memory has no mappings yet. */ - (!is_hardware_domain(currd) || d->tot_pages) ) + (!is_hardware_domain(currd) || domain_tot_pages(d)) ) domain_crash(d); /* XXX How to deal with existing mappings? */ } diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c index 7e1f563012..7f0d27c153 100644 --- a/xen/arch/x86/numa.c +++ b/xen/arch/x86/numa.c @@ -419,7 +419,7 @@ static void dump_numa(unsigned char key) { process_pending_softirqs(); - printk("Domain %u (total: %u):\n", d->domain_id, d->tot_pages); + printk("Domain %u (total: %u):\n", d->domain_id, domain_tot_pages(d)); for_each_online_node ( i ) page_num_node[i] = 0; diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c index 9a97cf4abf..5678da782d 100644 --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -110,8 +110,9 @@ static __init void setup_pv_physmap(struct domain *d, unsigned long pgtbl_pfn, while ( vphysmap_start < vphysmap_end ) { - if ( d->tot_pages + ((round_pgup(vphysmap_end) - vphysmap_start) - >> PAGE_SHIFT) + 3 > nr_pages ) + if ( domain_tot_pages(d) + + ((round_pgup(vphysmap_end) - vphysmap_start) >> PAGE_SHIFT) + + 3 > nr_pages ) panic("Dom0 allocation too small for initial P->M table\n"); if ( pl1e ) @@ -264,7 +265,7 @@ static struct page_info * __init alloc_chunk(struct domain *d, { struct page_info *pg2; - if ( d->tot_pages + (1 << order) > d->max_pages ) + if ( domain_tot_pages(d) + (1 << order) > d->max_pages ) continue; pg2 = alloc_domheap_pages(d, order, MEMF_exact_node | MEMF_no_scrub); if ( pg2 > page ) @@ -500,13 +501,13 @@ int __init dom0_construct_pv(struct domain *d, if ( page == NULL ) panic("Not enough RAM for domain 0 allocation\n"); alloc_spfn = mfn_x(page_to_mfn(page)); - alloc_epfn = alloc_spfn + d->tot_pages; + alloc_epfn = alloc_spfn + domain_tot_pages(d); if ( initrd_len ) { initrd_pfn = vinitrd_start ? (vinitrd_start - v_start) >> PAGE_SHIFT : - d->tot_pages; + domain_tot_pages(d); initrd_mfn = mfn = initrd->mod_start; count = PFN_UP(initrd_len); if ( d->arch.physaddr_bitsize && @@ -541,9 +542,9 @@ int __init dom0_construct_pv(struct domain *d, printk("PHYSICAL MEMORY ARRANGEMENT:\n" " Dom0 alloc.: %"PRIpaddr"->%"PRIpaddr, pfn_to_paddr(alloc_spfn), pfn_to_paddr(alloc_epfn)); - if ( d->tot_pages < nr_pages ) + if ( domain_tot_pages(d) < nr_pages ) printk(" (%lu pages to be allocated)", - nr_pages - d->tot_pages); + nr_pages - domain_tot_pages(d)); if ( initrd ) { mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT; @@ -755,7 +756,7 @@ int __init dom0_construct_pv(struct domain *d, snprintf(si->magic, sizeof(si->magic), "xen-3.0-x86_%d%s", elf_64bit(&elf) ? 64 : 32, parms.pae ? "p" : ""); - count = d->tot_pages; + count = domain_tot_pages(d); /* Set up the phys->machine table if not part of the initial mapping. */ if ( parms.p2m_base != UNSET_ADDR ) @@ -786,7 +787,7 @@ int __init dom0_construct_pv(struct domain *d, process_pending_softirqs(); } si->first_p2m_pfn = pfn; - si->nr_p2m_frames = d->tot_pages - count; + si->nr_p2m_frames = domain_tot_pages(d) - count; page_list_for_each ( page, &d->page_list ) { mfn = mfn_x(page_to_mfn(page)); @@ -804,15 +805,15 @@ int __init dom0_construct_pv(struct domain *d, process_pending_softirqs(); } } - BUG_ON(pfn != d->tot_pages); + BUG_ON(pfn != domain_tot_pages(d)); #ifndef NDEBUG alloc_epfn += PFN_UP(initrd_len) + si->nr_p2m_frames; #endif while ( pfn < nr_pages ) { - if ( (page = alloc_chunk(d, nr_pages - d->tot_pages)) == NULL ) + if ( (page = alloc_chunk(d, nr_pages - domain_tot_pages(d))) == NULL ) panic("Not enough RAM for DOM0 reservation\n"); - while ( pfn < d->tot_pages ) + while ( pfn < domain_tot_pages(d) ) { mfn = mfn_x(page_to_mfn(page)); #ifndef NDEBUG diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c index 4da0b2afff..c95652d1b8 100644 --- a/xen/arch/x86/pv/domain.c +++ b/xen/arch/x86/pv/domain.c @@ -173,7 +173,7 @@ int switch_compat(struct domain *d) BUILD_BUG_ON(offsetof(struct shared_info, vcpu_info) != 0); - if ( is_hvm_domain(d) || d->tot_pages != 0 ) + if ( is_hvm_domain(d) || domain_tot_pages(d) != 0 ) return -EACCES; if ( is_pv_32bit_domain(d) ) return 0; diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 8b819f56e5..bdc24bbd7c 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -191,7 +191,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info) xsm_security_domaininfo(d, info); - info->tot_pages = d->tot_pages; + info->tot_pages = domain_tot_pages(d); info->max_pages = d->max_pages; info->outstanding_pages = d->outstanding_pages; info->shr_pages = atomic_read(&d->shr_pages); diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 5536d282b9..8bee6b3b66 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -2261,7 +2261,7 @@ gnttab_transfer( * pages when it is dying. */ if ( unlikely(e->is_dying) || - unlikely(e->tot_pages >= e->max_pages) ) + unlikely(domain_tot_pages(e) >= e->max_pages) ) { spin_unlock(&e->page_alloc_lock); @@ -2271,7 +2271,7 @@ gnttab_transfer( else gdprintk(XENLOG_INFO, "Transferee d%d has no headroom (tot %u, max %u)\n", - e->domain_id, e->tot_pages, e->max_pages); + e->domain_id, domain_tot_pages(e), e->max_pages); gop.status = GNTST_general_error; goto unlock_and_copyback; diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c index f50490d0f3..87bd145374 100644 --- a/xen/common/keyhandler.c +++ b/xen/common/keyhandler.c @@ -271,7 +271,7 @@ static void dump_domains(unsigned char key) atomic_read(&d->pause_count)); printk(" nr_pages=%d xenheap_pages=%d shared_pages=%u paged_pages=%u " "dirty_cpus={%*pbl} max_pages=%u\n", - d->tot_pages, d->xenheap_pages, atomic_read(&d->shr_pages), + domain_tot_pages(d), d->xenheap_pages, atomic_read(&d->shr_pages), atomic_read(&d->paged_pages), CPUMASK_PR(d->dirty_cpumask), d->max_pages); printk(" handle=%02x%02x%02x%02x-%02x%02x-%02x%02x-" diff --git a/xen/common/memory.c b/xen/common/memory.c index c7d2bac452..bf464e8799 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -717,7 +717,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) /* * Pages in in_chunk_list is stolen without - * decreasing the tot_pages. If the domain is dying when + * decreasing domain_tot_pages(). If the domain is dying when * assign pages, we need decrease the count. For those pages * that has been assigned, it should be covered by * domain_relinquish_resources(). @@ -1267,7 +1267,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) switch ( op ) { case XENMEM_current_reservation: - rc = d->tot_pages; + rc = domain_tot_pages(d); break; case XENMEM_maximum_reservation: rc = d->max_pages; diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 919a270587..bbd3163909 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -518,8 +518,8 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages) goto out; } - /* disallow a claim not exceeding current tot_pages or above max_pages */ - if ( (pages <= d->tot_pages) || (pages > d->max_pages) ) + /* disallow a claim not exceeding domain_tot_pages() or above max_pages */ + if ( (pages <= domain_tot_pages(d)) || (pages > d->max_pages) ) { ret = -EINVAL; goto out; @@ -532,9 +532,9 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages) /* * Note, if domain has already allocated memory before making a claim - * then the claim must take tot_pages into account + * then the claim must take domain_tot_pages() into account */ - claim = pages - d->tot_pages; + claim = pages - domain_tot_pages(d); if ( claim > avail_pages ) goto out; @@ -2269,11 +2269,12 @@ int assign_pages( if ( !(memflags & MEMF_no_refcount) ) { - if ( unlikely((d->tot_pages + (1 << order)) > d->max_pages) ) + unsigned int tot_pages = domain_tot_pages(d) + (1 << order); + + if ( unlikely(tot_pages > d->max_pages) ) { gprintk(XENLOG_INFO, "Over-allocation for domain %u: " - "%u > %u\n", d->domain_id, - d->tot_pages + (1 << order), d->max_pages); + "%u > %u\n", d->domain_id, tot_pages, d->max_pages); rc = -E2BIG; goto out; } diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index cfdda6e2a8..126d0ff06e 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -553,8 +553,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_sharing_op_t); * * Note that a valid claim may be staked even after memory has been * allocated for a domain. In this case, the claim is not incremental, - * i.e. if the domain's tot_pages is 3, and a claim is staked for 10, - * only 7 additional pages are claimed. + * i.e. if the domain's total page count is 3, and a claim is staked + * for 10, only 7 additional pages are claimed. * * Caller must be privileged or the hypercall fails. */ diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 7c5c437247..1b6d7b941f 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -364,12 +364,18 @@ struct domain spinlock_t page_alloc_lock; /* protects all the following fields */ struct page_list_head page_list; /* linked list */ struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */ - unsigned int tot_pages; /* number of pages currently possesed */ - unsigned int xenheap_pages; /* # pages allocated from Xen heap */ - unsigned int outstanding_pages; /* pages claimed but not possessed */ - unsigned int max_pages; /* maximum value for tot_pages */ - atomic_t shr_pages; /* number of shared pages */ - atomic_t paged_pages; /* number of paged-out pages */ + + /* + * This field should only be directly accessed by domain_adjust_tot_pages() + * and the domain_tot_pages() helper function defined below. + */ + unsigned int tot_pages; + + unsigned int xenheap_pages; /* pages allocated from Xen heap */ + unsigned int outstanding_pages; /* pages claimed but not possessed */ + unsigned int max_pages; /* maximum value for domain_tot_pages() */ + atomic_t shr_pages; /* shared pages */ + atomic_t paged_pages; /* paged-out pages */ /* Scheduling. */ void *sched_priv; /* scheduler-specific data */ @@ -539,6 +545,12 @@ struct domain #endif }; +/* Return number of pages currently posessed by the domain */ +static inline unsigned int domain_tot_pages(const struct domain *d) +{ + return d->tot_pages; +} + /* Protect updates/reads (resp.) of domain_list and domain_hash. */ extern spinlock_t domlist_update_lock; extern rcu_read_lock_t domlist_read_lock;
This patch adds a new domain_tot_pages() inline helper function into sched.h, which will be needed by a subsequent patch. No functional change. NOTE: While modifying the comment for 'tot_pages' in sched.h this patch makes some cosmetic fixes to surrounding comments. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Paul Durrant <pdurrant@amazon.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Wei Liu <wl@xen.org> Cc: "Roger Pau Monné" <roger.pau@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Julien Grall <julien@xen.org> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Tim Deegan <tim@xen.org> v8: - New in v8 --- xen/arch/x86/domain.c | 2 +- xen/arch/x86/mm.c | 6 +++--- xen/arch/x86/mm/p2m-pod.c | 10 +++++----- xen/arch/x86/mm/shadow/common.c | 2 +- xen/arch/x86/msi.c | 2 +- xen/arch/x86/numa.c | 2 +- xen/arch/x86/pv/dom0_build.c | 25 +++++++++++++------------ xen/arch/x86/pv/domain.c | 2 +- xen/common/domctl.c | 2 +- xen/common/grant_table.c | 4 ++-- xen/common/keyhandler.c | 2 +- xen/common/memory.c | 4 ++-- xen/common/page_alloc.c | 15 ++++++++------- xen/include/public/memory.h | 4 ++-- xen/include/xen/sched.h | 24 ++++++++++++++++++------ 15 files changed, 60 insertions(+), 46 deletions(-)