linux-kernel - Re: [PATCH v1 01/10] perf/x86/uncore: fix initialization of cpumask

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABPqkBTJNrWwbnWazXV2g3m2MBmujp-mjod9Ujy3UMguv7Gp0g@mail.gmail.com>
Date:	Tue, 11 Feb 2014 12:12:21 +0100
From:	Stephane Eranian <eranian@...gle.com>
To:	"Yan, Zheng" <zheng.z.yan@...el.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"mingo@...e.hu" <mingo@...e.hu>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	"ak@...ux.intel.com" <ak@...ux.intel.com>
Subject: Re: [PATCH v1 01/10] perf/x86/uncore: fix initialization of cpumask

On Mon, Feb 10, 2014 at 3:49 AM, Yan, Zheng <zheng.z.yan@...el.com> wrote:
> why not something like below. I think it's simpler.
>
I don't like that too much. It assumes msr_uncores is initialized properly.
I have seen compiler complaining about missing default: case.

> ---
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index 29c2487..169ef4a 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -3799,9 +3799,6 @@ static int __init uncore_cpu_init(void)
>                         ivt_uncore_cbox.num_boxes = max_cores;
>                 msr_uncores = ivt_msr_uncores;
>                 break;
> -
> -       default:
> -               return 0;
>         }
>
>         ret = uncore_types_init(msr_uncores);
>
>
> Regards
> Yan, Zheng
>
>
> On 02/03/2014 08:55 PM, Stephane Eranian wrote:
>> On certain processors, the uncore PMU boxes may only be
>> msr-bsed or PCI-based. But in both cases, the cpumask,
>> suggesting on which CPUs to monitor to get full coverage
>> of the particular PMU, must be created.
>>
>> However with the current code base, the cpumask was only
>> created on processor which had at least one MSR-based
>> uncore PMU. This patch removes that restriction and
>> ensures the cpumask is created even when there is no
>> msr-based PMU. For instance, on SNB client where only
>> a PCI-based memory controller PMU is supported.
>>
>> Signed-off-by: Stephane Eranian <eranian@...gle.com>
>> ---
>>  arch/x86/kernel/cpu/perf_event_intel_uncore.c |   61 +++++++++++++++----------
>>  1 file changed, 37 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> index 29c2487..fe4255b 100644
>> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> @@ -3764,7 +3764,7 @@ static void __init uncore_cpu_setup(void *dummy)
>>
>>  static int __init uncore_cpu_init(void)
>>  {
>> -     int ret, cpu, max_cores;
>> +     int ret, max_cores;
>>
>>       max_cores = boot_cpu_data.x86_max_cores;
>>       switch (boot_cpu_data.x86_model) {
>> @@ -3808,29 +3808,6 @@ static int __init uncore_cpu_init(void)
>>       if (ret)
>>               return ret;
>>
>> -     get_online_cpus();
>> -
>> -     for_each_online_cpu(cpu) {
>> -             int i, phys_id = topology_physical_package_id(cpu);
>> -
>> -             for_each_cpu(i, &uncore_cpu_mask) {
>> -                     if (phys_id == topology_physical_package_id(i)) {
>> -                             phys_id = -1;
>> -                             break;
>> -                     }
>> -             }
>> -             if (phys_id < 0)
>> -                     continue;
>> -
>> -             uncore_cpu_prepare(cpu, phys_id);
>> -             uncore_event_init_cpu(cpu);
>> -     }
>> -     on_each_cpu(uncore_cpu_setup, NULL, 1);
>> -
>> -     register_cpu_notifier(&uncore_cpu_nb);
>> -
>> -     put_online_cpus();
>> -
>>       return 0;
>>  }
>>
>> @@ -3859,6 +3836,41 @@ static int __init uncore_pmus_register(void)
>>       return 0;
>>  }
>>
>> +static void uncore_cpumask_init(void)
>> +{
>> +     int cpu;
>> +
>> +     /*
>> +      * ony invoke once from msr or pci init code
>> +      */
>> +     if (!cpumask_empty(&uncore_cpu_mask))
>> +             return;
>> +
>> +     get_online_cpus();
>> +
>> +     for_each_online_cpu(cpu) {
>> +             int i, phys_id = topology_physical_package_id(cpu);
>> +
>> +             for_each_cpu(i, &uncore_cpu_mask) {
>> +                     if (phys_id == topology_physical_package_id(i)) {
>> +                             phys_id = -1;
>> +                             break;
>> +                     }
>> +             }
>> +             if (phys_id < 0)
>> +                     continue;
>> +
>> +             uncore_cpu_prepare(cpu, phys_id);
>> +             uncore_event_init_cpu(cpu);
>> +     }
>> +     on_each_cpu(uncore_cpu_setup, NULL, 1);
>> +
>> +     register_cpu_notifier(&uncore_cpu_nb);
>> +
>> +     put_online_cpus();
>> +}
>> +
>> +
>>  static int __init intel_uncore_init(void)
>>  {
>>       int ret;
>> @@ -3877,6 +3889,7 @@ static int __init intel_uncore_init(void)
>>               uncore_pci_exit();
>>               goto fail;
>>       }
>> +     uncore_cpumask_init();
>>
>>       uncore_pmus_register();
>>       return 0;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/