lkml.org 
[lkml]   [2014]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 08/11] perf/x86/intel: Add Intel Cache QoS Monitoring support
    On Wed, 24 Sep, at 09:40:10AM, Andi Kleen wrote:
    > Matt Fleming <matt@console-pimps.org> writes:
    > >
    > > diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
    > > index 7e1fd4e08552..8abb18fbcd13 100644
    > > --- a/arch/x86/kernel/cpu/Makefile
    > > +++ b/arch/x86/kernel/cpu/Makefile
    > > @@ -38,7 +38,7 @@ obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_knc.o perf_event_p4.o
    > > obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o
    > > obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_uncore.o perf_event_intel_uncore_snb.o
    > > obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_uncore_snbep.o perf_event_intel_uncore_nhmex.o
    > > -obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_rapl.o
    > > +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_rapl.o perf_event_intel_cqm.o
    >
    > What's missing to be able to make this a module?

    Not sure, that's not something I'd thought of. I simply copied every
    other PMU driver in this directory.

    But as an experiment I tried it, and the only thing that appears to be
    necessary is adding EXPORT_SYMBOL_GPL() to events_sysfs_show.

    > > +
    > > + /*
    > > + * Is @cpu a designated cqm reader?
    > > + */
    > > + if (!cpumask_test_and_clear_cpu(cpu, &cqm_cpumask))
    > > + return;
    > > +
    > > + for_each_online_cpu(i) {
    >
    > Likely possible cpus to avoid races? Otherwise you'll need more locking.

    I was under the impression that CPU_DOWN_PREPARE notifiers were
    serialized against cpu hotplug. And reading the code, that does appear
    to be so.

    What race have you got in mind?

    > > +static int __init intel_cqm_init(void)
    > > +{
    > > + int i, cpu, ret;
    > > +
    > > + if (!cpu_has(&boot_cpu_data, X86_FEATURE_CQM_OCCUP_LLC))
    > > + return -ENODEV;
    >
    > This should use cpufeature.h

    What? Please be more explicit.

    > > +
    > > + cqm_l3_scale = boot_cpu_data.x86_cache_occ_scale;
    > > +
    > > + /*
    > > + * It's possible that not all resources support the same number
    > > + * of RMIDs. Instead of making scheduling much more complicated
    > > + * (where we have to match a task's RMID to a cpu that supports
    > > + * that many RMIDs) just find the minimum RMIDs supported across
    > > + * all cpus.
    > > + *
    > > + * Also, check that the scales match on all cpus.
    > > + */
    > > + for_each_online_cpu(cpu) {
    >
    > And this should take the cpu hotplug lock (although it may be
    > latent at this point if it's only running at early initializion)

    Good catch, this is racy. I'll fix this up.

    > But in fact what good is the test then if you only
    > every likely check cpu #0?

    We don't, we check every online cpu, not just cpu 0.

    --
    Matt Fleming, Intel Open Source Technology Center


    \
     
     \ /
      Last update: 2014-09-24 23:01    [W:4.955 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site