[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJvTdKm-GThyvWsv8meGDPxymF+2PV0EH=Ny6crO+8MfS5QeLg@mail.gmail.com>
Date: Thu, 27 Nov 2025 12:45:42 -0500
From: Len Brown <lenb@...nel.org>
To: David Arcari <darcari@...hat.com>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] tools/power turbostat: avoid an infinite loop of restarts
I guess it is sort of an art form upon malfunction to decide whether
it is fatal or not.
I suppose if we know that a counter used to work, and it stops working, then we
are responding to a configuration change. We don't want to count a
certain number
of configuration changes as a fatal error.
A more useful heuristic may be to recognize when there are failures
without any intervening successes,
and if those happen many times per second. Likely that is the
infinite loop case we're looking for...
On Tue, Nov 25, 2025 at 4:51 PM David Arcari <darcari@...hat.com> wrote:
>
>
> I see. Perhaps this isn't fixable then. I'll take another look.
>
> -DA
>
> On 11/25/25 2:12 PM, Len Brown wrote:
> > this patch introduces a limit of 10-restarts per turbostat lifetime,
> > down from infinity.
> >
> > some turbostat invocations span multiple uses of cpu online/offline --
> > so this limit will not fly.
> >
> > On Tue, Nov 18, 2025 at 10:58 AM David Arcari <darcari@...hat.com> wrote:
> >>
> >> There are some error cases where turbostat will attempt to reinitialize
> >> by calling the re_initialize() function. The code attempts to avoid
> >> an infinite loop by checking the value of 'restarted' in one case, but
> >> not others. It should be checked in all cases of restart. Additonally,
> >> the 'restarted' is reset to zero at the start of the loop which also
> >> needs to be removed.
> >>
> >> Signed-off-by: David Arcari <darcari@...hat.com>
> >> Cc: Len Brown <lenb@...nel.org>
> >> Cc: linux-kernel@...r.kernel.org
> >> ---
> >> tools/power/x86/turbostat/turbostat.c | 10 +++++-----
> >> 1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> >> index 584b0f7f9067..5567b9ecd516 100644
> >> --- a/tools/power/x86/turbostat/turbostat.c
> >> +++ b/tools/power/x86/turbostat/turbostat.c
> >> @@ -6722,7 +6722,11 @@ void turbostat_loop()
> >> set_my_sched_priority(-20);
> >>
> >> restart:
> >> - restarted++;
> >> + if (restarted++ > 10) {
> >> + if (!retval)
> >> + retval = -1;
> >> + exit(retval);
> >> + }
> >>
> >> snapshot_proc_sysfs_files();
> >> retval = for_all_cpus(get_counters, EVEN_COUNTERS);
> >> @@ -6730,13 +6734,9 @@ void turbostat_loop()
> >> if (retval < -1) {
> >> exit(retval);
> >> } else if (retval == -1) {
> >> - if (restarted > 10) {
> >> - exit(retval);
> >> - }
> >> re_initialize();
> >> goto restart;
> >> }
> >> - restarted = 0;
> >> done_iters = 0;
> >> gettimeofday(&tv_even, (struct timezone *)NULL);
> >>
> >> --
> >> 2.51.0
> >>
> >>
> >
> >
>
--
Len Brown, Intel Open Source Technology Center
Powered by blists - more mailing lists