[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> On 16.01.15 at 18:07, <boris.ostrovsky@xxxxxxxxxx> wrote:
> 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).

I initially was, but then reconsidered Andrew's arguments and
became unconvinced.

> 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.

I'm not sure why this has a version associated with it. In any
event that version didn't ever change since its introduction, by
means of which the interface has been stable.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.