[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [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(), - 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 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |