From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A3BCC4360C for ; Fri, 27 Sep 2019 18:59:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6C11921655 for ; Fri, 27 Sep 2019 18:59:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1569610795; bh=E73g80IzW6M1EitSeL0eIccYNXP+iG5cDrk7F+CAOI4=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-ID:From; b=zcDLloCV/ZC8DphHPPqE2oLAJLsO3PQShJC7UtoaeIQ0tDTWMYQvLoUS0uyYsNzCC 2+0JFPa7jlXqRpv9VD4BxEIMMiwUlTLqypuQ24cwox5VB/hXJZu2SaQ99fkfyEu7db xh+jAgxcPMp84YdFBqUJLHsp55SchpgWoUU6p574= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728177AbfI0S7z (ORCPT ); Fri, 27 Sep 2019 14:59:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:58528 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728171AbfI0S7y (ORCPT ); Fri, 27 Sep 2019 14:59:54 -0400 Received: from [192.168.1.112] (c-24-9-64-241.hsd1.co.comcast.net [24.9.64.241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id EA403207FF; Fri, 27 Sep 2019 18:59:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1569610793; bh=E73g80IzW6M1EitSeL0eIccYNXP+iG5cDrk7F+CAOI4=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=lbk0CZYN7l4OBvnl8OAlF/JVsFiAJJq3vwVUD0lLmU/yX6kt3w+k1OWLrXJHWINsf 2geCKyLrIXXlCvJYibE1NgOXKyQ5AAk37lHdNwsXf6eEQJwTKA+9FYoPHekUNzxJZL RJDe79uBC2uqbPQqBCrp+bjjQAy16oa1v2OhntUA= Subject: Re: [PATCH 1/2] Modify cpupower to schedule itself on cores it is reading MSRs from To: "Natarajan, Janakarajan" , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" Cc: Thomas Renninger , Pu Wen , Borislav Petkov , Allison Randal , Thomas Gleixner , Kate Stewart , Greg Kroah-Hartman , Richard Fontana , shuah References: <20190918163445.129103-1-Janakarajan.Natarajan@amd.com> From: shuah Message-ID: Date: Fri, 27 Sep 2019 12:59:52 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190918163445.129103-1-Janakarajan.Natarajan@amd.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On 9/18/19 10:34 AM, Natarajan, Janakarajan wrote: > Modify cpupower to schedule itself on each of the cpus in the system and > then get the APERF/MPERF register values. > > This is advantageous because an IPI is not generated when a read_msr() is > executed on the local logical CPU thereby reducing the chance of having > APERF and MPERF being out of sync. Somehow this doesn't read right. Is this that you are trying to avoid APERF and MPERF being out of sync with this change? This description is rather confusing. > > Signed-off-by: Janakarajan Natarajan > --- > .../utils/idle_monitor/mperf_monitor.c | 38 ++++++++++++++----- > 1 file changed, 28 insertions(+), 10 deletions(-) > > diff --git a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c > index 44806a6dae11..8b072e39c897 100644 > --- a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c > +++ b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > > #include > > @@ -86,15 +87,33 @@ static int mperf_get_tsc(unsigned long long *tsc) > return ret; > } > > +static int get_aperf_mperf(int cpu, unsigned long long *aval, > + unsigned long long *mval) > +{ > + cpu_set_t set; > + int ret; > + > + CPU_ZERO(&set); > + CPU_SET(cpu, &set); > + if (sched_setaffinity(getpid(), sizeof(set), &set) == -1) { > + dprint("Could not migrate to cpu: %d\n", cpu); > + return 1; > + } > + > + ret = read_msr(cpu, MSR_APERF, aval); > + ret |= read_msr(cpu, MSR_MPERF, mval); > + > + return ret; > +} > + > static int mperf_init_stats(unsigned int cpu) > { > - unsigned long long val; > + unsigned long long aval, mval; > int ret; > > - ret = read_msr(cpu, MSR_APERF, &val); > - aperf_previous_count[cpu] = val; > - ret |= read_msr(cpu, MSR_MPERF, &val); > - mperf_previous_count[cpu] = val; > + ret = get_aperf_mperf(cpu, &aval, &mval); get_aperf_mperf() could return error right? It returns 1 when sched_setaffinity() fails. Shouldn't the return value checked, instead of using aval and mval? > + aperf_previous_count[cpu] = aval; > + mperf_previous_count[cpu] = mval; > is_valid[cpu] = !ret; > > return 0; > @@ -102,13 +121,12 @@ static int mperf_init_stats(unsigned int cpu) > > static int mperf_measure_stats(unsigned int cpu) > { > - unsigned long long val; > + unsigned long long aval, mval; > int ret; > > - ret = read_msr(cpu, MSR_APERF, &val); > - aperf_current_count[cpu] = val; > - ret |= read_msr(cpu, MSR_MPERF, &val); > - mperf_current_count[cpu] = val; > + ret = get_aperf_mperf(cpu, &aval, &mval); Same comments as above here. > + aperf_current_count[cpu] = aval; > + mperf_current_count[cpu] = mval; > is_valid[cpu] = !ret; > > return 0; > thanks, -- Shuah