Re: [Xen-devel] [PATCH v2 2/4] sysctl: Make XEN_SYSCTL_topologyinfo sysctl a little more efficient

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 01/16/2015 11:57 AM, Andrew Cooper wrote:
> On 16/01/15 16:45, Jan Beulich wrote:
> > > > > On 16.01.15 at 17:38, <Ian.Campbell@xxxxxxxxxx> wrote:
> > > On Fri, 2015-01-16 at 16:34 +0000, Jan Beulich wrote:
> > > > > > > On 16.01.15 at 17:16, <Ian.Campbell@xxxxxxxxxx> wrote:
> > > > > On Fri, 2015-01-16 at 16:06 +0000, Jan Beulich wrote:
> > > > > > > > > On 16.01.15 at 16:56, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > > > > > > On 01/07/2015 04:12 AM, Jan Beulich wrote:
> > > > > > > > > > > On 06.01.15 at 14:41, <andrew.cooper3@xxxxxxxxxx> wrote:
> > > > > > > > > On 06/01/15 02:18, Boris Ostrovsky wrote:
> > > > > > > > > > Instead of copying data for each field in xen_sysctl_topologyinfo
> > > > > separately
> > > > > > > > > > put cpu/socket/node into a single structure and do a single copy for each
> > > > > > > > > > processor.
> > > > > > > > > >
> > > > > > > > > > There is also no need to copy whole op to user at the end, max_cpu_index
> > > is
> > > > > > > > > > sufficient
> > > > > > > > > >
> > > > > > > > > > Rename xen_sysctl_topologyinfo and XEN_SYSCTL_topologyinfo to reflect the
> > > > > > > fact
> > > > > > > > > > that these are used for CPU topology. Subsequent patch will add support
> > > for
> > > > > > > > > > PCI topology sysctl.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > > > > > > > > If we are going to change the hypercall, then can we see about making it
> > > > > > > > > a stable interface (i.e. not a sysctl/domctl)?  There are non-toolstack
> > > > > > > > > components which might want/need access to this information.  (i.e. I am
> > > > > > > > > still looking for a reasonable way to get this information from Xen in
> > > > > > > > > hwloc)
> > > > > > > > In which case leaving the sysctl alone and just adding a new non-sysctl
> > > > > > > > interface should be considered.
> > > > > > > (Sorry for late reply)
> > > > > > >
> > > > > > > Would a platform op be an option here or do you prefer a whole new
> > > > > > > hypercall?
> > > > > >  From an abstract pov a platform op would be fine, but iirc you had
> > > > > > a need for preempting, which doesn't work well for that hypercall.
> > > > > > A whole new one seems overkill too. Perhaps slightly bending what
> > > > > > physdevop-s are used for might be an option...
> > > > > Unlike sysctls, physdevop-s are exposed to/stable for dom0 too aren't
> > > > > they?
> > > > Sure, just like platformop-s. What is it I'm not understanding you
> > > > try to point out with your question?
> > > By moving from a sysctl to a physdev op we would then have to declare
> > > the interface stable and lose the ability to change it in the future,
> > > and since it didn't look like the intention here was to expose to dom0
> > > (make more efficient didn't imply that at least) that seems a bit
> > > unnecessary.
> > The conversion from sysctl was something Andrew had asked for.
> > After some consideration I had actually indicated I'm not really
> > convinced of the motivation he gave, but I don't think I heard
> > back on this. So _if_ we want to expose this to other than the
> > tool stack, then _of course_ the interface can't be changed at
> > our liking anymore (this stability was part of what Andrew wanted
> > iirc).
> The real question is whether this information is useful to anything
> other than toolstack-like entities.
>
> I have been partially dissuaded from my stance of "yes" on this point.
> While it is possible that there are toolstack-like entities which want
> this information, there is almost nothing useful which could be done
> without other toolstack gubbins in place.
So then you are OK with keeping this in sysctl? (I actually must have 
misunderstood Jan's position on that from earlier discussion --- I 
thought he was advocating for moving away from sysctl as well).
Also, a side question --- how is platform ops interface considered 
stable if it is versioned? I may have a different understanding of what 
"stable" means.
-boris


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