Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1424808 > unrolled thread

regression caused by 08f511fd41c3 ("cpufreq: Reduce cpufreq_update_util() overhead a bit")

Back to article view | Back to linux.kernel

Contents

  regression caused by 08f511fd41c3 ("cpufreq: Reduce  cpufreq_update_util() overhead a bit") Jisheng Zhang <jszhang@marvell.com> - 2016-06-17 10:40 +0200
    Re: regression caused by 08f511fd41c3 ("cpufreq: Reduce  cpufreq_update_util() overhead a bit") Jisheng Zhang <jszhang@marvell.com> - 2016-06-17 10:50 +0200
      Re: regression caused by 08f511fd41c3 ("cpufreq: Reduce  cpufreq_update_util() overhead a bit") "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-17 15:10 +0200
        Re: regression caused by 08f511fd41c3 ("cpufreq: Reduce  cpufreq_update_util() overhead a bit") "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-06-17 15:20 +0200
          Re: regression caused by 08f511fd41c3 ("cpufreq: Reduce  cpufreq_update_util() overhead a bit") Peter Zijlstra <peterz@infradead.org> - 2016-06-17 16:10 +0200
            Re: regression caused by 08f511fd41c3 ("cpufreq: Reduce  cpufreq_update_util() overhead a bit") "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-17 17:40 +0200
            RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set  utilization update hook too early") Jisheng Zhang <jszhang@marvell.com> - 2016-06-17 17:40 +0200
              Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set  utilization update hook too early") "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-17 17:50 +0200
                Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set  utilization update hook too early") "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-17 17:50 +0200
                  RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set  utilization update hook too early") Jisheng Zhang <jszhang@marvell.com> - 2016-06-17 18:00 +0200
                    RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set  utilization update hook too early") Jisheng Zhang <jszhang@marvell.com> - 2016-06-17 18:20 +0200
              RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early") "Doug Smythies" <dsmythies@telus.net> - 2016-06-23 18:20 +0200

#1424808 — regression caused by 08f511fd41c3 ("cpufreq: Reduce cpufreq_update_util() overhead a bit")

Dear all,

I found one regression: In an idle system, wakeups/s (reported by powertop)
is increased a lot, e.g on a intel snb 4 core platform, the wakeup event
number is increased from 8 wakeups/s to 24 wakeup/s. bisect points to
this commit. I could send detailed bisect log if it's wanted.


Thanks,
Jisheng

[toc] | [next] | [standalone]

#1424812

Dear all,

On Fri, 17 Jun 2016 16:30:23 +0800 Jisheng Zhang wrote:

> Dear all,
> 
> I found one regression: In an idle system, wakeups/s (reported by powertop)
> is increased a lot, e.g on a intel snb 4 core platform, the wakeup event
> number is increased from 8 wakeups/s to 24 wakeup/s. bisect points to
> this commit. I could send detailed bisect log if it's wanted.
> 

more information maybe useful: after the commit, the top two wakeup source
are

        Process        [rcu_sched]

        Timer          tick_sched_timer

thanks

[toc] | [prev] | [next] | [standalone]

#1425053

On Fri, Jun 17, 2016 at 10:40 AM, Jisheng Zhang <jszhang@marvell.com> wrote:
> Dear all,
>
> On Fri, 17 Jun 2016 16:30:23 +0800 Jisheng Zhang wrote:
>
>> Dear all,
>>
>> I found one regression: In an idle system, wakeups/s (reported by powertop)
>> is increased a lot, e.g on a intel snb 4 core platform, the wakeup event
>> number is increased from 8 wakeups/s to 24 wakeup/s. bisect points to
>> this commit. I could send detailed bisect log if it's wanted.
>>
>
> more information maybe useful: after the commit, the top two wakeup source
> are
>
>         Process        [rcu_sched]
>
>         Timer          tick_sched_timer

And what was there before the commit?

Granted, I'm not seeing this on my systems.

Paul, Peter, any ideas about what may be going on here?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1425062

On Fri, Jun 17, 2016 at 03:09:36PM +0200, Rafael J. Wysocki wrote:
> On Fri, Jun 17, 2016 at 10:40 AM, Jisheng Zhang <jszhang@marvell.com> wrote:
> > Dear all,
> >
> > On Fri, 17 Jun 2016 16:30:23 +0800 Jisheng Zhang wrote:
> >
> >> Dear all,
> >>
> >> I found one regression: In an idle system, wakeups/s (reported by powertop)
> >> is increased a lot, e.g on a intel snb 4 core platform, the wakeup event
> >> number is increased from 8 wakeups/s to 24 wakeup/s. bisect points to
> >> this commit. I could send detailed bisect log if it's wanted.
> >>
> >
> > more information maybe useful: after the commit, the top two wakeup source
> > are
> >
> >         Process        [rcu_sched]
> >
> >         Timer          tick_sched_timer
> 
> And what was there before the commit?
> 
> Granted, I'm not seeing this on my systems.
> 
> Paul, Peter, any ideas about what may be going on here?

Looks to me like this commit moved some code from synchronize_rcu() to
synchronize_sched().  Assuming that this is a CONFIG_PREEMPT=y system,
might there have been a decrease in the wakeups from the rcu_preempt
kthread?

							Thanx, Paul

[toc] | [prev] | [next] | [standalone]

#1425124

On Fri, Jun 17, 2016 at 06:16:51AM -0700, Paul E. McKenney wrote:
> > Paul, Peter, any ideas about what may be going on here?
> 
> Looks to me like this commit moved some code from synchronize_rcu() to
> synchronize_sched().  Assuming that this is a CONFIG_PREEMPT=y system,
> might there have been a decrease in the wakeups from the rcu_preempt
> kthread?

The 'funny' thing is though; those synchronize thingies are only reached
when we change cpufreq policy, so things like:

  for i in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor ; do echo performance > $i ; done

Something which is hardly possible when idle. Weird.

[toc] | [prev] | [next] | [standalone]

#1425228

On Fri, Jun 17, 2016 at 4:03 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jun 17, 2016 at 06:16:51AM -0700, Paul E. McKenney wrote:
>> > Paul, Peter, any ideas about what may be going on here?
>>
>> Looks to me like this commit moved some code from synchronize_rcu() to
>> synchronize_sched().  Assuming that this is a CONFIG_PREEMPT=y system,
>> might there have been a decrease in the wakeups from the rcu_preempt
>> kthread?
>
> The 'funny' thing is though; those synchronize thingies are only reached
> when we change cpufreq policy, so things like:
>
>   for i in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor ; do echo performance > $i ; done

Or if you change min or max for a policy and the driver is intel_pstate.

> Something which is hardly possible when idle. Weird.

Well, exactly.

Jisheng, do you use intel_pstate or acpi-cpufreq as the scaling driver?

[toc] | [prev] | [next] | [standalone]

#1425239 — RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

Hi all,

First of all, sorry for top post, only webmail is available now.

Second, sorry again for report incorrect commit, I were too tired this morning so I remember the wrong commit.  The regression is caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early"), so I update the email title.

here is the bisect log:

# good: [9735a22799b9214d17d3c231fe377fc852f042e9] Linux 4.6-rc2
git bisect good 9735a22799b9214d17d3c231fe377fc852f042e9

# bad: [bf16200689118d19de1b8d2a3c314fc21f5dc7bb] Linux 4.6-rc3
git bisect bad bf16200689118d19de1b8d2a3c314fc21f5dc7bb

# good: [839a3f765728cdca0057a12e2dc0bf669ac1c22e] Merge branch 'for-linus-4.6' of git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs
git bisect good 839a3f765728cdca0057a12e2dc0bf669ac1c22e

# bad: [63b106a87dd84283e21aa2ce476732633eaab11d] Merge tag 'md/4.6-rc2-fix' of git://git.kernel.org/pub/scm/linux/kernel/git/shli/md
git bisect bad 63b106a87dd84283e21aa2ce476732633eaab11d

# good: [30d237a6c2e9be1bb816fe8e787b88fd7aad833b] Merge tag 'mac80211-for-davem-2016-04-06' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211
git bisect good 30d237a6c2e9be1bb816fe8e787b88fd7aad833b

# bad: [fa81e66ec8648f62e96e95e53db2ea95a4b57b26] Merge branches 'pm-cpufreq', 'pm-cpuidle' and 'acpi-cppc'
git bisect bad fa81e66ec8648f62e96e95e53db2ea95a4b57b26

# good: [08820546e4c30c84d0a1f1a49df055e1719c07ea] intel_idle: Propagate hot plug errors.
git bisect good 08820546e4c30c84d0a1f1a49df055e1719c07ea

# bad: [b318556479cc923970a79d6c2311138581c0db83] cpufreq: dt: Drop stale comment
git bisect bad b318556479cc923970a79d6c2311138581c0db83

# bad: [febce40febcff3ccdb33f63456ffc4cfc61640c8] intel_pstate: Avoid extra invocation of intel_pstate_sample()
git bisect bad febce40febcff3ccdb33f63456ffc4cfc61640c8

# bad: [bb6ab52f2befe1fb29ac198f27d8a6aadf510f81] intel_pstate: Do not set utilization update hook too early
git bisect bad bb6ab52f2befe1fb29ac198f27d8a6aadf510f81

# first bad commit: [bb6ab52f2befe1fb29ac198f27d8a6aadf510f81] intel_pstate: Do not set utilization update hook too early


________________________________________
From: Peter Zijlstra [peterz@infradead.org]
Sent: Friday, June 17, 2016 22:03
To: Paul E. McKenney
Cc: Rafael J. Wysocki; Jisheng Zhang; Rafael J. Wysocki; Viresh Kumar; linux-pm@vger.kernel.org; Linux Kernel Mailing List
Subject: Re: regression caused by 08f511fd41c3 ("cpufreq: Reduce cpufreq_update_util() overhead a bit")

On Fri, Jun 17, 2016 at 06:16:51AM -0700, Paul E. McKenney wrote:
> > Paul, Peter, any ideas about what may be going on here?
>
> Looks to me like this commit moved some code from synchronize_rcu() to
> synchronize_sched().  Assuming that this is a CONFIG_PREEMPT=y system,
> might there have been a decrease in the wakeups from the rcu_preempt
> kthread?

The 'funny' thing is though; those synchronize thingies are only reached
when we change cpufreq policy, so things like:

  for i in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor ; do echo performance > $i ; done

Something which is hardly possible when idle. Weird.

[toc] | [prev] | [next] | [standalone]

#1425243 — Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

On Fri, Jun 17, 2016 at 5:32 PM, Jisheng Zhang <jszhang@marvell.com> wrote:
> Hi all,
>
> First of all, sorry for top post, only webmail is available now.
>
> Second, sorry again for report incorrect commit, I were too tired this morning so I remember the wrong commit.  The regression is caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early"), so I update the email title.

OK, that makes much more sense. :-)

And

4578ee7e1def intel_pstate: Avoid unnecessary synchronize_sched()
during initialization

is not sufficient I suppose?

[toc] | [prev] | [next] | [standalone]

#1425249 — Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

On Fri, Jun 17, 2016 at 5:40 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Jun 17, 2016 at 5:32 PM, Jisheng Zhang <jszhang@marvell.com> wrote:
>> Hi all,
>>
>> First of all, sorry for top post, only webmail is available now.
>>
>> Second, sorry again for report incorrect commit, I were too tired this morning so I remember the wrong commit.  The regression is caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early"), so I update the email title.
>
> OK, that makes much more sense. :-)
>
> And
>
> 4578ee7e1def intel_pstate: Avoid unnecessary synchronize_sched()
> during initialization
>
> is not sufficient I suppose?

I mean, it is not sufficient to reduce the number of wakeups again?

[toc] | [prev] | [next] | [standalone]

#1425269 — RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

Dear Rafael,

I used intel_pstate.

I just tested v4.7-rc3, which should include commit 4578ee7e1def intel_pstate: Avoid
unnecessary synchronize_sched() during initialization, I can still get the same 
wakeups, so it's not sufficient.

Another clue maybe helpful, I found these top2 wakeup/s is consistent, e.g
rcu_sched always gives about 10 wakeups/s, and tick_sched_timer gave
5.6-5.9 wakeups/s, 

            144.3 µs/s      10.0        Process        [rcu_sched]

            193.9 µs/s       5.7        Timer          tick_sched_timer

Thanks,
Jisheng

________________________________________
From:  Rafael J. Wysocki 
Sent: Friday, June 17, 2016 23:42
To: Jisheng Zhang
Cc: Peter Zijlstra; Paul E. McKenney; Rafael J. Wysocki; Viresh Kumar; linux-pm@vger.kernel.org; Linux Kernel Mailing List
Subject: Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

On Fri, Jun 17, 2016 at 5:40 PM, Rafael J. Wysocki  wrote:
> On Fri, Jun 17, 2016 at 5:32 PM, Jisheng Zhang  wrote:
>> Hi all,
>>
>> First of all, sorry for top post, only webmail is available now.
>>
>> Second, sorry again for report incorrect commit, I were too tired this morning so I remember the wrong commit.  The regression is caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early"), so I update the email title.
>
> OK, that makes much more sense. :-)
>
> And
>
> 4578ee7e1def intel_pstate: Avoid unnecessary synchronize_sched()
> during initialization
>
> is not sufficient I suppose?

I mean, it is not sufficient to reduce the number of wakeups again?

[toc] | [prev] | [next] | [standalone]

#1425301 — RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

Dear all,

If using acpi-cpufreq instead, v4.6, v4.6-rc3, v4.7-rc3 can't reproduce the issue. It seems
only intel_pstate is impacted.

Thanks,
Jisheng
________________________________________
From: Jisheng Zhang
Sent: Friday, June 17, 2016 23:53
To: Rafael J. Wysocki
Cc: Peter Zijlstra; Paul E. McKenney; Rafael J. Wysocki; Viresh Kumar; linux-pm@vger.kernel.org; Linux Kernel Mailing List
Subject: RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

Dear Rafael,

I used intel_pstate.

I just tested v4.7-rc3, which should include commit 4578ee7e1def intel_pstate: Avoid
unnecessary synchronize_sched() during initialization, I can still get the same
wakeups, so it's not sufficient.

Another clue maybe helpful, I found these top2 wakeup/s is consistent, e.g
rcu_sched always gives about 10 wakeups/s, and tick_sched_timer gave
5.6-5.9 wakeups/s,

            144.3 µs/s      10.0        Process        [rcu_sched]

            193.9 µs/s       5.7        Timer          tick_sched_timer

Thanks,
Jisheng

________________________________________
From:  Rafael J. Wysocki
Sent: Friday, June 17, 2016 23:42
To: Jisheng Zhang
Cc: Peter Zijlstra; Paul E. McKenney; Rafael J. Wysocki; Viresh Kumar; linux-pm@vger.kernel.org; Linux Kernel Mailing List
Subject: Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

On Fri, Jun 17, 2016 at 5:40 PM, Rafael J. Wysocki  wrote:
> On Fri, Jun 17, 2016 at 5:32 PM, Jisheng Zhang  wrote:
>> Hi all,
>>
>> First of all, sorry for top post, only webmail is available now.
>>
>> Second, sorry again for report incorrect commit, I were too tired this morning so I remember the wrong commit.  The regression is caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early"), so I update the email title.
>
> OK, that makes much more sense. :-)
>
> And
>
> 4578ee7e1def intel_pstate: Avoid unnecessary synchronize_sched()
> during initialization
>
> is not sufficient I suppose?

I mean, it is not sufficient to reduce the number of wakeups again?

[toc] | [prev] | [next] | [standalone]

#1429960 — RE: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early")

On 2016.06.17 08:32 Jisheng Zhang wrote:

> Second, sorry again for report incorrect commit, 
> I were too tired this morning so I remember the wrong commit.
> The regression is caused by bb6ab52f2bef ("intel_pstate: Do not
> set utilization update hook too early"), so I update the email title.

Summary:

Isn't running powertop itself a significant contributor here?

When running powertop, the system is no longer "idle".
Powertop has a significant effect on a system that achieves such a quite "idle state".
If one uses a kernel that includes the later commit:
"intel_pstate: Avoid extra invocation of intel_pstate_sample()"
then there are many skipped samples when running powertop.

To some extent, some increase in wakeups per second, is what was supposed to
happen with the recent changes, and is what solved the very long standing issue
of failing to raise the CPU frequency when high enough periodic loads just so
happened to be idle on jiffy boundaries. Admittedly, I am somewhat confusing
wakeups per second with number of passes through the intel_pstate driver
per second here.

Details:

Note 1: On my system (server, no GUI), I could only get down to an "idle" of around
20 wakeups / second, after turning off Samba, mysql, apache, cron.

Using kernel 4.7-rc3 (and then later 4.7-rc4), I observed significant impact when running powertop.
(I also isolated this to the same commit, bb6ab52f2bef)
As a function of sample time:
Sample time (seconds)	Events/second
300				~20
200				~20
100				~22
 50				~29
 30				~33
 20				~44
  3				~155

Instead of using powertop, if I observe timer stats manually I get ~20 wakeups / second.
If I use an older kernel (4.4 ish) I get ~20 wakeups per second.

If I leave mysql running, chosen because while idling it has a fairly high number
of events per second, no increase in wakeups per second is observed before or after the commit
in question here (it is always around 25 Events / second).

Using a kernel that includes the commit
"intel_pstate: Avoid extra invocation of intel_pstate_sample()"
and running powertop with a sample interval of 5 seconds, just to increase
the number of events per second, then intel_pstate samples are skipped at
a rate of about 200 per minute, and some trace data makes no sense at all. Besides the
skipped samples, sometimes the new pstate is set to maximum for no apparent reason.
It seems that powertop messes with the minimum frequency, sometimes setting it
to the maximum turbo value (presumably at a rate of about 100 per minute,
causing one skip when setting it high and one skip when it is restored) Example:

The following script was run while running powertop:

#!/bin/dash

echo "watch minimum frequency. Doug Smythies 2016.06.23"
echo "I think powertop messes with the minimum frequency. Try to catch it."
echo "If I use watch, with -g, it exits but doesn't leave the display showing the numbers."
echo
  cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_min_freq > watch_min
while [ 1 ];
do
  echo "." >> watch_min
# Don't hog too much CPU, even though it makes it harder to get a hit
  sleep 0.2
  cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_min_freq >> watch_min
done

In the resulting file several occurrences similar to the below are observed:

.
1600000
3800000   <<<<<<<<<< ????
1600000
1600000
1600000
1600000
1600000
1600000
.
1600000
1600000
1600000
1600000
1600000
1600000
1600000
1600000
.

On my, otherwise idle, system package power goes up by about 1 watt while running powertop
(with a sample time of 5 seconds) (from ~3.9 to ~4.9 watts).

> here is the bisect log:
>
> # good: [9735a22799b9214d17d3c231fe377fc852f042e9] Linux 4.6-rc2
> git bisect good 9735a22799b9214d17d3c231fe377fc852f042e9
> # bad: [bf16200689118d19de1b8d2a3c314fc21f5dc7bb] Linux 4.6-rc3
> git bisect bad bf16200689118d19de1b8d2a3c314fc21f5dc7bb
> # good: [839a3f765728cdca0057a12e2dc0bf669ac1c22e] Merge branch 'for-linus-4.6' of
git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs
> git bisect good 839a3f765728cdca0057a12e2dc0bf669ac1c22e
> # bad: [63b106a87dd84283e21aa2ce476732633eaab11d] Merge tag 'md/4.6-rc2-fix' of
git://git.kernel.org/pub/scm/linux/kernel/git/shli/md
> git bisect bad 63b106a87dd84283e21aa2ce476732633eaab11d
> # good: [30d237a6c2e9be1bb816fe8e787b88fd7aad833b] Merge tag 'mac80211-for-davem-2016-04-06' of
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211
> git bisect good 30d237a6c2e9be1bb816fe8e787b88fd7aad833b
> # bad: [fa81e66ec8648f62e96e95e53db2ea95a4b57b26] Merge branches 'pm-cpufreq', 'pm-cpuidle' and 'acpi-cppc'
> git bisect bad fa81e66ec8648f62e96e95e53db2ea95a4b57b26
> # good: [08820546e4c30c84d0a1f1a49df055e1719c07ea] intel_idle: Propagate hot plug errors.
> git bisect good 08820546e4c30c84d0a1f1a49df055e1719c07ea
> # bad: [b318556479cc923970a79d6c2311138581c0db83] cpufreq: dt: Drop stale comment
> git bisect bad b318556479cc923970a79d6c2311138581c0db83
> # bad: [febce40febcff3ccdb33f63456ffc4cfc61640c8] intel_pstate: Avoid extra invocation of intel_pstate_sample()
> git bisect bad febce40febcff3ccdb33f63456ffc4cfc61640c8
> # bad: [bb6ab52f2befe1fb29ac198f27d8a6aadf510f81] intel_pstate: Do not set utilization update hook too early
> git bisect bad bb6ab52f2befe1fb29ac198f27d8a6aadf510f81
> # first bad commit: [bb6ab52f2befe1fb29ac198f27d8a6aadf510f81] intel_pstate: Do not set utilization update hook too early

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web