Re: [Xen-devel] [PATCH 1/4] xen: report how much memory a domain has on each NUMA node

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On mer, 2014-03-05 at 15:50 +0100, Juergen Gross wrote:
> On 05.03.2014 15:36, Dario Faggioli wrote:

> > diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> > index 7cf610a..96bf326 100644
> > --- a/xen/common/domctl.c
> > +++ b/xen/common/domctl.c
> > @@ -574,6 +574,51 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> > u_domctl)

> > +        spin_lock(&d->page_alloc_lock);
> > +        page_list_for_each(page, &d->page_list)
> > +        {
> > +            node = phys_to_nid((paddr_t)page_to_mfn(page) << PAGE_SHIFT);
> > +            /* For nodes that are offline, don't touch the counter */
> > +            if ( node <= max_node_index && node_online(node) )
> > +                memkb_on_node[node]++;
> > +        }
> 
> This loop will run quite a long time for huge domains. Wouldn't it be better
> to do the accounting during page allocation?
> 
Se the reply to Jan (and feel free to chime in, of course. :-P)

> > +        spin_unlock(&d->page_alloc_lock);
> > +
> > +        for ( node = 0; node <= max_node_index; node++ )
> > +        {
> > +            memkb_on_node[node] <<= (PAGE_SHIFT-10);
> 
> If you already use a 64 bit element you could use bytes as unit.
> 
Yeah, I was trying to be consistent with other calls and interfaces, but
there is pretty much everything out there with respect to this: uint32,
unsigned int, unsigned long... :-O

At least in libxl we have (tools/libxl/libxl_types.idl):

#
# Specific integer types
#

MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")

And hence:

typedef struct libxl_domain_build_info {
    int max_vcpus;
    libxl_bitmap avail_vcpus;
    libxl_bitmap cpumap;
    libxl_bitmap nodemap;
    libxl_defbool numa_placement;
    libxl_tsc_mode tsc_mode;
    uint64_t max_memkb;
    uint64_t target_memkb;
    uint64_t video_memkb;
    uint64_t shadow_memkb;
    ...

Having looked at this again, I wonder whether sticking to Kbs and
switching to uint32 wouldn't be the best solution...

> > +/* XEN_DOMCTL_numainfo */
> > +struct xen_domctl_numainfo {
> > +    /*
> > +     * IN: maximum addressable entry in the caller-provided arrays.
> > +     * OUT: minimum between the maximum addressable entry in the
> > +     *      caller-provided arrays and largest online node identifier
> > +     *      in the system.
> > +     */
> > +    uint32_t max_node_index;
> 
> Add explicit padding?
> 
Let's see. As per the reply to Jan, this interface may change.

Anyway, I'll do so if it doesn't.

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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