From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v6 1/5] sysctl: Make XEN_SYSCTL_numainfo a little more efficient Date: Wed, 8 Apr 2015 17:31:16 +0100 Message-ID: <552557D4.6050003@citrix.com> References: <1428358329-8466-1-git-send-email-boris.ostrovsky@oracle.com> <1428358329-8466-2-git-send-email-boris.ostrovsky@oracle.com> <5523FFF3.8050106@citrix.com> <55240C83.2050803@oracle.com> <55240E2D.4050900@citrix.com> <552413D5.9040705@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <552413D5.9040705@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky , ian.jackson@eu.citrix.com, ian.campbell@citrix.com, wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, dgdegra@tycho.nsa.gov, jbeulich@suse.com, keir@xen.org Cc: elena.ufimtseva@oracle.com, dario.faggioli@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 07/04/15 18:28, Boris Ostrovsky wrote: > On 04/07/2015 01:04 PM, Andrew Cooper wrote: >> On 07/04/15 17:57, Boris Ostrovsky wrote: >>> On 04/07/2015 12:04 PM, Andrew Cooper wrote: >>>> On 06/04/15 23:12, Boris Ostrovsky wrote: >>>>> A number of changes to XEN_SYSCTL_numainfo interface: >>>>> >>>>> * Make sysctl NUMA topology query use fewer copies by combining some >>>>> fields into a single structure and copying distances for each >>>>> node >>>>> in a single copy. >>>>> * NULL meminfo and distance handles are a request for maximum number >>>>> of nodes (num_nodes). If those handles are valid and num_nodes is >>>>> is smaller than the number of nodes in the system then >>>>> -ENOBUFS is >>>>> returned (and correct num_nodes is provided) >>>>> * Instead of using max_node_index for passing number of nodes keep >>>>> this >>>>> value in num_nodes: almost all uses of max_node_index required >>>>> adding >>>>> or subtracting one to eventually get to number of nodes anyway. >>>>> * Replace INVALID_NUMAINFO_ID with XEN_INVALID_MEM_SZ and add >>>>> XEN_INVALID_NODE_DIST. >>>>> >>>>> Signed-off-by: Boris Ostrovsky >>>>> Acked-by: Ian Campbell >>>> This subtly changes the behaviour of XEN_SYSCTL_numainfo with >>>> regards to >>>> NULL guest handles. >>>> >>>> Previously, a caller was able to select which information they >>>> wanted by >>>> choosing which guest handles were non-NULL. >>>> >>>> With the new semantics, the caller must pass both ni->meminfo and >>>> ni->distance to get either bit of information. Each >>>> copy_to_guest_offset() should be gated on a !guest_handle_is_null() >>>> so a >>>> caller can request meminfo information without distance information. >>> >>> Currently the caller, in fact, can have either of three pointers >>> (node_to_memsize, node_to_memfree or node_to_node_distance) as NULL >>> and the hypervisor will fill whichever pointer is valid. Because I put >>> the first two together into a struct we are already changing behavior >>> in that regard. Not to mention that having all three as NULL now has >>> new meaning as well. >>> >>> I thought that either both pointers should be valid or neither. If >>> people disagree I can change this. >> I have a usecase (hwloc) which wants meminfo but not distance. In such >> a case, the inner for loop can be skipped which is an advantage for Xen. > > Are you planning on calling sysctls directly from your hwloc > extension? Why not go via libxl which is a stable interface AFAIUI. I did when experimenting. Not sure going forwards, but probably libxl for API reasons. Still, for the sake of two guest_handle_is_null() checks, I think we should keep the ability to choose a subset of the information returned. Consider some statistics daemon wanting to periodically poll for memory usage on certain nodes without wanting the distance matrix each time. ~Andrew