Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <2439425b90fb305cc1938fca22e22505851e9c08.camel@kepstin.ca>
Subject: Re: [PATCH v2] tools/power turbostat: Fix RAPL summary collection
 on AMD processors
From:   Calvin Walton <calvin.walton@kepstin.ca>
To:     Terry Bowman <terry.bowman@amd.com>, lenb@kernel.org,
        yu.c.chen@intel.com, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org, bp@suse.de
Cc:     wei.huang2@amd.com
Date:   Tue, 20 Apr 2021 09:32:36 -0400
In-Reply-To: <20210419195812.147710-1-terry.bowman@amd.com>
References: <20210419195812.147710-1-terry.bowman@amd.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.38.2 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 2021-04-19 at 14:58 -0500, Terry Bowman wrote:
> Turbostat fails to correctly collect and display RAPL summary
> information
> on Family 17h and 19h AMD processors. Running turbostat on these
> processors
> returns immediately. If turbostat is working correctly then RAPL
> summary
> data is displayed until the user provided command completes. If a
> command
> is not provided by the user then turbostat is designed to
> continuously
> display RAPL information until interrupted.
> 
> The issue is due to offset_to_idx() and idx_to_offset() missing
> support for
> AMD MSR addresses/offsets. offset_to_idx()'s switch statement is
> missing
> cases for AMD MSRs and idx_to_offset() does not include a path to
> return
> AMD MSR(s) for any idx.

Ah, when I was looking at Boris's patch, it lead me to one more spot
that was missed with the off_t type - inside the function
update_msr_sum(), the offset is also stored in a variable of type int,
and as a result the background update for AMD's MSR when turbostat is
used with long durations won't work. (It's skipped because of the
offset < 0 check).

-- 
Calvin Walton <calvin.walton@kepstin.ca>