Re: [Xen-devel] [PATCH v5 4/8] sysctl: Make XEN_SYSCTL_numainfo a little more efficient

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.03.15 at 22:54, <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -274,53 +274,62 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
> u_sysctl)
>  
>      case XEN_SYSCTL_numainfo:
>      {
> -        uint32_t i, j, max_node_index, last_online_node;
> +        uint32_t i, j, num_nodes;

unsigned int please.

>          xen_sysctl_numainfo_t *ni = &op->u.numainfo;
>  
> -        last_online_node = last_node(node_online_map);
> -        max_node_index = min_t(uint32_t, ni->max_node_index, 
> last_online_node);
> -        ni->max_node_index = last_online_node;
> +        num_nodes = last_node(node_online_map) + 1;
>  
> -        for ( i = 0; i <= max_node_index; i++ )
> +        if ( !guest_handle_is_null(ni->meminfo) &&
> +             !guest_handle_is_null(ni->distance) )
>          {
> -            if ( !guest_handle_is_null(ni->node_to_memsize) )
> +            if ( ni->num_nodes < num_nodes )
>              {
> -                uint64_t memsize = node_online(i) ?
> -                                   node_spanned_pages(i) << PAGE_SHIFT : 0ul;
> -                if ( copy_to_guest_offset(ni->node_to_memsize, i, &memsize, 
> 1) )
> -                    break;
> -            }
> -            if ( !guest_handle_is_null(ni->node_to_memfree) )
> -            {
> -                uint64_t memfree = node_online(i) ?
> -                                   avail_node_heap_pages(i) << PAGE_SHIFT : 
> 0ul;
> -                if ( copy_to_guest_offset(ni->node_to_memfree, i, &memfree, 
> 1) )
> -                    break;
> +                ret = -ENOBUFS;
> +                i = num_nodes;
>              }
>  
> -            if ( !guest_handle_is_null(ni->node_to_node_distance) )
> +            for ( i = 0; i < num_nodes; i++ )
>              {
> -                for ( j = 0; j <= max_node_index; j++)
> +                xen_sysctl_meminfo_t meminfo;
> +                uint32_t distance[MAX_NUMNODES];

I don't think it is a good idea to put such an array on the stack. Since
this is serialized with a lock anyway, please make this static.


> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -494,34 +494,41 @@ typedef struct xen_sysctl_cputopoinfo 
> xen_sysctl_cputopoinfo_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cputopoinfo_t);
>  
>  /* XEN_SYSCTL_numainfo */
> -#define INVALID_NUMAINFO_ID (~0U)
> +#define XEN_INVALID_MEM_SZ     (~0U)
> +#define XEN_INVALID_NODE_DIST  (~0U)
> +
> +struct xen_sysctl_meminfo {
> +    uint64_t memsize;
> +    uint64_t memfree;
> +};
> +typedef struct xen_sysctl_meminfo xen_sysctl_meminfo_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_meminfo_t);
> +
> +/*
> + * IN:
> + *  - NULL 'meminfo' and 'distance'  handles is a request for maximun
> + *    'num_nodes'.

Please bring code and comment in line (the "and" here contradicts
the implicit "or" in the code).

Jan


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