Re: [Xen-devel] [PATCH v12 6/9] x86: collect global QoS monitoring information

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.08.14 at 10:26, <dongxiao.xu@xxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Friday, July 04, 2014 8:14 PM
>> To: Xu, Dongxiao
>> Cc: andrew.cooper3@xxxxxxxxxx; Ian.Campbell@xxxxxxxxxx;
>> George.Dunlap@xxxxxxxxxxxxx; Ian.Jackson@xxxxxxxxxxxxx;
>> stefano.stabellini@xxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxx;
>> konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx; keir@xxxxxxx 
>> Subject: Re: [PATCH v12 6/9] x86: collect global QoS monitoring information
>> 
>> >>> On 04.07.14 at 10:34, <dongxiao.xu@xxxxxxxxx> wrote:
>> > +        case XEN_SYSCTL_PQOS_MONITOR_get_l3_cache_size:
>> > +            sysctl->u.pqos_monitor_op.data =
>> boot_cpu_data.x86_cache_size;
>> > +            break;
>> 
>> ISTR having asked before - is boot_cpu_data.x86_cache_size really
>> always the L3 cache size?
> 
> On current machines with CQM enabled, boot_cpu_data.x86_cache_size reflects 
> the L3 cache size, unless if later we have more level of caches (maybe L4, 
> L5, etc... just guessing).
> I think the current solution is okay, unless you prefer to have another 
> round CPUID enumeration here.

And as long as there aren't going to be CPUs without any L3.

In particular for future systems, if anyone ever altered
init_intel_cacheinfo(), I don't think (s)he'll remember that the QOS
code relies on x86_cache_size to be L3. So yes, I think you ought
to specifically look for the L3 cache size here.

>> > +        case XEN_SYSCTL_PQOS_MONITOR_get_socket_cpu:
>> > +        {
>> > +            unsigned int i, cpu;
>> > +            unsigned int socket = sysctl->u.pqos_monitor_op.data;
>> > +
>> > +            for ( i = 0; i < nr_cpu_ids; i++ )
>> > +            {
>> > +                if ( cpu_to_socket(i) < 0 || cpu_to_socket(i) != socket )
>> > +                    continue;
>> > +                cpu = cpumask_any(per_cpu(cpu_core_mask, i));
>> > +                if ( cpu < nr_cpu_ids )
>> > +                {
>> > +                    sysctl->u.pqos_monitor_op.data = cpu;
>> > +                    break;
>> > +                }
>> > +            }
>> > +
>> > +            if ( i == nr_cpu_ids )
>> > +                ret = -ESRCH;
>> > +        }
>> > +        break;
>> 
>> It should be possible for the tools to get this information by using
>> XEN_SYSCTL_topologyinfo.
> 
> I noticed that the XEN_SYSCTL_topologyinfo hypercall is heavy loaded, which 
> passes more data between hypervisor and guest, while we only need one "int" 
> result.

To me that's rather lame an excuse for adding a new sysctl.

Jan


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