On Tue, 3 Oct 2023, srinivas pandruvada wrote:
> On Tue, 2023-10-03 at 16:10 +0300, Ilpo Järvinen wrote:
> > On Mon, 2 Oct 2023, Srinivas Pandruvada wrote:
> >
> > > When the current uncore frequency can't be read, don't create
> > > attribute
> > > "current_freq_khz" as any read will fail later. Some user space
> > > applications like turbostat fail to continue with the failure. So,
> > > check
> > > error during attribute creation.
> > >
> > > Fixes: 8a54e2253e4c ("platform/x86/intel-uncore-freq: Uncore
> > > frequency control via TPMI")
> >
> > Hi,
> >
> > Thanks for the update but that commit id looks bogus, or where the
> > value
> > is used w/o error check?
>
> commit 8a54e2253e4c25e5b61c9a9bee157bb52da5d432
> Author: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> Date: Thu Apr 20 15:05:14 2023 -0700
>
> platform/x86/intel-uncore-freq: Uncore frequency control via TPMI
>
>
> This is the commit exposed the issue. This is not the commit which
> changed the code in question.
>
>
> I can add also
> Fixes: dbce412a7733 ("platform/x86/intel-uncore-freq: Split common and
> enumeration part")
>
> But the change even before that as this commit just reorganized code
> but because of change of folders, that will look like correct commit.
I never thought dbce412a7733 is being fixed here, it's just a refactor
moving code around like you say.
But how about 414eef27283a ("platform/x86/intel/uncore-freq: Display
uncore current frequency") which actually adds the code line you're now
fixing. What was broken before it? All I see is the one call in
show_perf_status_freq_khz() but that's checking for errors.
--
i.
