diff mbox

[REGRESSION] 774ac8b7eff6 ("Thermal: initialize thermal zone device correctly") causes performance drop

Message ID 56EC811C.6020404@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Zhang Rui
Headers show

Commit Message

Laura Abbott March 18, 2016, 10:28 p.m. UTC 
(bringing this back to the main thread)

On 03/16/2016 05:20 PM, Pandruvada, Srinivas wrote:
> On Wed, 2016-03-16 at 17:00 -0700, Laura Abbott wrote:
>> On 03/16/2016 03:46 PM, Greg Kroah-Hartman wrote:
>>> On Wed, Mar 16, 2016 at 03:27:57PM -0700, Laura Abbott wrote:
>>>> Hi,
>>>>
>>>> Fedora received a bug report (https://bugzilla.redhat.com/show_bu
>>>> g.cgi?id=1317190)
>>>> of a major performance drop on various bench marks and general
>>>> system
>>>> sluggishness with the 4.4.4 kernel update. The benchmarks were
>>>> showing
>>>> a reduction to about 18% performance (not minor).
>>>>
>>>> Bisection showed the first bad commit was
>>>>
>>>> commit 774ac8b7eff69e0786970157de2157e68b22f456
>>>> Author: Zhang Rui <rui.zhang@intel.com>
>>>> Date:   Fri Oct 30 16:31:47 2015 +0800
>>>>
>>>>       Thermal: initialize thermal zone device correctly
>>>>       commit bb431ba26c5cd0a17c941ca6c3a195a3a6d5d461 upstream.
>>>>       After thermal zone device registered, as we have not read
>>>> any
>>>>       temperature before, thus tz->temperature should not be 0,
>>>>       which actually means 0C, and thermal trend is not available.
>>>>       In this case, we need specially handling for the first
>>>>       thermal_zone_device_update().
>>>>       Both thermal core framework and step_wise governor is
>>>>       enhanced to handle this. And since the step_wise governor
>>>>       is the only one that uses trends, so it's the only thermal
>>>>       governor that needs to be updated.
>>>>       Tested-by: Manuel Krause <manuelkrause@netscape.net>
>>>>       Tested-by: szegad <szegadlo@poczta.onet.pl>
>>>>       Tested-by: prash <prash.n.rao@gmail.com>
>>>>       Tested-by: amish <ammdispose-arch@yahoo.com>
>>>>       Tested-by: Matthias <morpheusxyz123@yahoo.de>
>>>>       Reviewed-by: Javi Merino <javi.merino@arm.com>
>>>>       Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>>>>       Signed-off-by: Chen Yu <yu.c.chen@intel.com>
>>>>       Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.or
>>>> g>
>>>>
>>>>
>>>>
>>>> Reverting this plus to other commits in the series (a67208e94d94
>>>> "Thermal: handle thermal zone device properly during system
>>>> sleep"
>>>> and 27f356149d59 "Thermal: do thermal zone update after a cooling
>>>> device registered") confirmed the performance was back to normal.
>>>>
>>>> Bugzilla has the full discussion but this comment from one of the
>>>> reporters sums it up:
>>>>
>>>> "In 4.4.3 and prior, my 2.40 MHz processor would fluctuate
>>>> between
>>>> 1000 and 3400 MHz.  In 4.4.4, the processor would fluctuate
>>>> between
>>>> 400 and 700 MHz, according to /proc/cpuinfo.
>>>>
>>>> Setting /sys/devices/system/cpu/cpufreq/policy0/scaling_governor
>>>> to
>>>> performance, instead of the default "powersave" forces the CPU to
>>>> 2400 MHz, and improves performance greatly, but still not to the
>>>> same level as in 4.4.3."
>>>>
>>>> Any ideas?
>>>
>>> Is this same "slowdown" also seen in 4.5?
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>
>> Yes, the same issue is seen on 4.5 according to the reporter.
> What does it show here when performance drops?
> grep . /sys/devices/system/cpu/intel_pstate/*
>
> Is the problem still occurs if you set
> /sys/class/thermal/thermal_zone*/mode to "disabled"
>
> Thanks,
> Srinivas
>

A separate thread was started which gave this insight:

"I think
the problem is your device has a passive trip temp of 0
/sys/class/thermal/thermal_zone0/trip_point_2_temp:0
/sys/class/thermal/thermal_zone0/trip_point_2_type:passive

Which triggers a false throttle = true. I think we should this trip as
invalid in the case of
if (tz->temperature >= trip_temp) {} check
in thermal_zone_trip_update()."

So would something like the following work?


(completely untested, no idea if I'm even close)

Thanks,
Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Pandruvada, Srinivas March 18, 2016, 11:36 p.m. UTC | #1 
T24gRnJpLCAyMDE2LTAzLTE4IGF0IDE1OjI4IC0wNzAwLCBMYXVyYSBBYmJvdHQgd3JvdGU6DQo+
IChicmluZ2luZyB0aGlzIGJhY2sgdG8gdGhlIG1haW4gdGhyZWFkKQ0KPiANCj4gT24gMDMvMTYv
MjAxNiAwNToyMCBQTSwgUGFuZHJ1dmFkYSwgU3Jpbml2YXMgd3JvdGU6DQo+ID4gT24gV2VkLCAy
MDE2LTAzLTE2IGF0IDE3OjAwIC0wNzAwLCBMYXVyYSBBYmJvdHQgd3JvdGU6DQo+ID4gPiBPbiAw
My8xNi8yMDE2IDAzOjQ2IFBNLCBHcmVnIEtyb2FoLUhhcnRtYW4gd3JvdGU6DQo+ID4gPiA+IE9u
IFdlZCwgTWFyIDE2LCAyMDE2IGF0IDAzOjI3OjU3UE0gLTA3MDAsIExhdXJhIEFiYm90dCB3cm90
ZToNCj4gPiA+ID4gPiBIaSwNCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBGZWRvcmEgcmVjZWl2ZWQg
YSBidWcgcmVwb3J0IChodHRwczovL2J1Z3ppbGxhLnJlZGhhdC5jb20vc2hvDQo+ID4gPiA+ID4g
d19idQ0KPiA+ID4gPiA+IGcuY2dpP2lkPTEzMTcxOTApDQo+ID4gPiA+ID4gb2YgYSBtYWpvciBw
ZXJmb3JtYW5jZSBkcm9wIG9uIHZhcmlvdXMgYmVuY2ggbWFya3MgYW5kDQo+ID4gPiA+ID4gZ2Vu
ZXJhbA0KPiA+ID4gPiA+IHN5c3RlbQ0KPiA+ID4gPiA+IHNsdWdnaXNobmVzcyB3aXRoIHRoZSA0
LjQuNCBrZXJuZWwgdXBkYXRlLiBUaGUgYmVuY2htYXJrcw0KPiA+ID4gPiA+IHdlcmUNCj4gPiA+
ID4gPiBzaG93aW5nDQo+ID4gPiA+ID4gYSByZWR1Y3Rpb24gdG8gYWJvdXQgMTglIHBlcmZvcm1h
bmNlIChub3QgbWlub3IpLg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IEJpc2VjdGlvbiBzaG93ZWQg
dGhlIGZpcnN0IGJhZCBjb21taXQgd2FzDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gY29tbWl0IDc3
NGFjOGI3ZWZmNjllMDc4Njk3MDE1N2RlMjE1N2U2OGIyMmY0NTYNCj4gPiA+ID4gPiBBdXRob3I6
IFpoYW5nIFJ1aSA8cnVpLnpoYW5nQGludGVsLmNvbT4NCj4gPiA+ID4gPiBEYXRlOsKgwqDCoEZy
aSBPY3QgMzAgMTY6MzE6NDcgMjAxNSArMDgwMA0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IMKgwqDC
oMKgwqDCoFRoZXJtYWw6IGluaXRpYWxpemUgdGhlcm1hbCB6b25lIGRldmljZSBjb3JyZWN0bHkN
Cj4gPiA+ID4gPiDCoMKgwqDCoMKgwqBjb21taXQgYmI0MzFiYTI2YzVjZDBhMTdjOTQxY2E2YzNh
MTk1YTNhNmQ1ZDQ2MQ0KPiA+ID4gPiA+IHVwc3RyZWFtLg0KPiA+ID4gPiA+IMKgwqDCoMKgwqDC
oEFmdGVyIHRoZXJtYWwgem9uZSBkZXZpY2UgcmVnaXN0ZXJlZCwgYXMgd2UgaGF2ZSBub3QNCj4g
PiA+ID4gPiByZWFkDQo+ID4gPiA+ID4gYW55DQo+ID4gPiA+ID4gwqDCoMKgwqDCoMKgdGVtcGVy
YXR1cmUgYmVmb3JlLCB0aHVzIHR6LT50ZW1wZXJhdHVyZSBzaG91bGQgbm90IGJlDQo+ID4gPiA+
ID4gMCwNCj4gPiA+ID4gPiDCoMKgwqDCoMKgwqB3aGljaCBhY3R1YWxseSBtZWFucyAwQywgYW5k
IHRoZXJtYWwgdHJlbmQgaXMgbm90DQo+ID4gPiA+ID4gYXZhaWxhYmxlLg0KPiA+ID4gPiA+IMKg
wqDCoMKgwqDCoEluIHRoaXMgY2FzZSwgd2UgbmVlZCBzcGVjaWFsbHkgaGFuZGxpbmcgZm9yIHRo
ZSBmaXJzdA0KPiA+ID4gPiA+IMKgwqDCoMKgwqDCoHRoZXJtYWxfem9uZV9kZXZpY2VfdXBkYXRl
KCkuDQo+ID4gPiA+ID4gwqDCoMKgwqDCoMKgQm90aCB0aGVybWFsIGNvcmUgZnJhbWV3b3JrIGFu
ZCBzdGVwX3dpc2UgZ292ZXJub3IgaXMNCj4gPiA+ID4gPiDCoMKgwqDCoMKgwqBlbmhhbmNlZCB0
byBoYW5kbGUgdGhpcy4gQW5kIHNpbmNlIHRoZSBzdGVwX3dpc2UNCj4gPiA+ID4gPiBnb3Zlcm5v
cg0KPiA+ID4gPiA+IMKgwqDCoMKgwqDCoGlzIHRoZSBvbmx5IG9uZSB0aGF0IHVzZXMgdHJlbmRz
LCBzbyBpdCdzIHRoZSBvbmx5DQo+ID4gPiA+ID4gdGhlcm1hbA0KPiA+ID4gPiA+IMKgwqDCoMKg
wqDCoGdvdmVybm9yIHRoYXQgbmVlZHMgdG8gYmUgdXBkYXRlZC4NCj4gPiA+ID4gPiDCoMKgwqDC
oMKgwqBUZXN0ZWQtYnk6IE1hbnVlbCBLcmF1c2UgPG1hbnVlbGtyYXVzZUBuZXRzY2FwZS5uZXQ+
DQo+ID4gPiA+ID4gwqDCoMKgwqDCoMKgVGVzdGVkLWJ5OiBzemVnYWQgPHN6ZWdhZGxvQHBvY3p0
YS5vbmV0LnBsPg0KPiA+ID4gPiA+IMKgwqDCoMKgwqDCoFRlc3RlZC1ieTogcHJhc2ggPHByYXNo
Lm4ucmFvQGdtYWlsLmNvbT4NCj4gPiA+ID4gPiDCoMKgwqDCoMKgwqBUZXN0ZWQtYnk6IGFtaXNo
IDxhbW1kaXNwb3NlLWFyY2hAeWFob28uY29tPg0KPiA+ID4gPiA+IMKgwqDCoMKgwqDCoFRlc3Rl
ZC1ieTogTWF0dGhpYXMgPG1vcnBoZXVzeHl6MTIzQHlhaG9vLmRlPg0KPiA+ID4gPiA+IMKgwqDC
oMKgwqDCoFJldmlld2VkLWJ5OiBKYXZpIE1lcmlubyA8amF2aS5tZXJpbm9AYXJtLmNvbT4NCj4g
PiA+ID4gPiDCoMKgwqDCoMKgwqBTaWduZWQtb2ZmLWJ5OiBaaGFuZyBSdWkgPHJ1aS56aGFuZ0Bp
bnRlbC5jb20+DQo+ID4gPiA+ID4gwqDCoMKgwqDCoMKgU2lnbmVkLW9mZi1ieTogQ2hlbiBZdSA8
eXUuYy5jaGVuQGludGVsLmNvbT4NCj4gPiA+ID4gPiDCoMKgwqDCoMKgwqBTaWduZWQtb2ZmLWJ5
OiBHcmVnIEtyb2FoLUhhcnRtYW4gPGdyZWdraEBsaW51eGZvdW5kYXRpDQo+ID4gPiA+ID4gb24u
b3INCj4gPiA+ID4gPiBnPg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IA0KPiA+
ID4gPiA+IFJldmVydGluZyB0aGlzIHBsdXMgdG8gb3RoZXIgY29tbWl0cyBpbiB0aGUgc2VyaWVz
DQo+ID4gPiA+ID4gKGE2NzIwOGU5NGQ5NA0KPiA+ID4gPiA+ICJUaGVybWFsOiBoYW5kbGUgdGhl
cm1hbCB6b25lIGRldmljZSBwcm9wZXJseSBkdXJpbmcgc3lzdGVtDQo+ID4gPiA+ID4gc2xlZXAi
DQo+ID4gPiA+ID4gYW5kIDI3ZjM1NjE0OWQ1OSAiVGhlcm1hbDogZG8gdGhlcm1hbCB6b25lIHVw
ZGF0ZSBhZnRlciBhDQo+ID4gPiA+ID4gY29vbGluZw0KPiA+ID4gPiA+IGRldmljZSByZWdpc3Rl
cmVkIikgY29uZmlybWVkIHRoZSBwZXJmb3JtYW5jZSB3YXMgYmFjayB0bw0KPiA+ID4gPiA+IG5v
cm1hbC4NCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBCdWd6aWxsYSBoYXMgdGhlIGZ1bGwgZGlzY3Vz
c2lvbiBidXQgdGhpcyBjb21tZW50IGZyb20gb25lIG9mDQo+ID4gPiA+ID4gdGhlDQo+ID4gPiA+
ID4gcmVwb3J0ZXJzIHN1bXMgaXQgdXA6DQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gIkluIDQuNC4z
IGFuZCBwcmlvciwgbXkgMi40MCBNSHogcHJvY2Vzc29yIHdvdWxkIGZsdWN0dWF0ZQ0KPiA+ID4g
PiA+IGJldHdlZW4NCj4gPiA+ID4gPiAxMDAwIGFuZCAzNDAwIE1Iei7CoMKgSW4gNC40LjQsIHRo
ZSBwcm9jZXNzb3Igd291bGQgZmx1Y3R1YXRlDQo+ID4gPiA+ID4gYmV0d2Vlbg0KPiA+ID4gPiA+
IDQwMCBhbmQgNzAwIE1IeiwgYWNjb3JkaW5nIHRvIC9wcm9jL2NwdWluZm8uDQo+ID4gPiA+ID4g
DQo+ID4gPiA+ID4gU2V0dGluZw0KPiA+ID4gPiA+IC9zeXMvZGV2aWNlcy9zeXN0ZW0vY3B1L2Nw
dWZyZXEvcG9saWN5MC9zY2FsaW5nX2dvdmVybm9yDQo+ID4gPiA+ID4gdG8NCj4gPiA+ID4gPiBw
ZXJmb3JtYW5jZSwgaW5zdGVhZCBvZiB0aGUgZGVmYXVsdCAicG93ZXJzYXZlIiBmb3JjZXMgdGhl
DQo+ID4gPiA+ID4gQ1BVIHRvDQo+ID4gPiA+ID4gMjQwMCBNSHosIGFuZCBpbXByb3ZlcyBwZXJm
b3JtYW5jZSBncmVhdGx5LCBidXQgc3RpbGwgbm90IHRvDQo+ID4gPiA+ID4gdGhlDQo+ID4gPiA+
ID4gc2FtZSBsZXZlbCBhcyBpbiA0LjQuMy4iDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gQW55IGlk
ZWFzPw0KPiA+ID4gPiANCj4gPiA+ID4gSXMgdGhpcyBzYW1lICJzbG93ZG93biIgYWxzbyBzZWVu
IGluIDQuNT8NCj4gPiA+ID4gDQo+ID4gPiA+IHRoYW5rcywNCj4gPiA+ID4gDQo+ID4gPiA+IGdy
ZWcgay1oDQo+ID4gPiA+IA0KPiA+ID4gDQo+ID4gPiBZZXMsIHRoZSBzYW1lIGlzc3VlIGlzIHNl
ZW4gb24gNC41IGFjY29yZGluZyB0byB0aGUgcmVwb3J0ZXIuDQo+ID4gV2hhdCBkb2VzIGl0IHNo
b3cgaGVyZSB3aGVuIHBlcmZvcm1hbmNlIGRyb3BzPw0KPiA+IGdyZXAgLiAvc3lzL2RldmljZXMv
c3lzdGVtL2NwdS9pbnRlbF9wc3RhdGUvKg0KPiA+IA0KPiA+IElzIHRoZSBwcm9ibGVtIHN0aWxs
IG9jY3VycyBpZiB5b3Ugc2V0DQo+ID4gL3N5cy9jbGFzcy90aGVybWFsL3RoZXJtYWxfem9uZSov
bW9kZSB0byAiZGlzYWJsZWQiDQo+ID4gDQo+ID4gVGhhbmtzLA0KPiA+IFNyaW5pdmFzDQo+ID4g
DQo+IA0KPiBBIHNlcGFyYXRlIHRocmVhZCB3YXMgc3RhcnRlZCB3aGljaCBnYXZlIHRoaXMgaW5z
aWdodDoNCj4gDQo+ICJJIHRoaW5rDQo+IHRoZSBwcm9ibGVtIGlzIHlvdXIgZGV2aWNlIGhhcyBh
IHBhc3NpdmUgdHJpcCB0ZW1wIG9mIDANCj4gL3N5cy9jbGFzcy90aGVybWFsL3RoZXJtYWxfem9u
ZTAvdHJpcF9wb2ludF8yX3RlbXA6MA0KPiAvc3lzL2NsYXNzL3RoZXJtYWwvdGhlcm1hbF96b25l
MC90cmlwX3BvaW50XzJfdHlwZTpwYXNzaXZlDQo+IA0KPiBXaGljaCB0cmlnZ2VycyBhIGZhbHNl
IHRocm90dGxlID0gdHJ1ZS4gSSB0aGluayB3ZSBzaG91bGQgdGhpcyB0cmlwDQo+IGFzDQo+IGlu
dmFsaWQgaW4gdGhlIGNhc2Ugb2YNCj4gaWYgKHR6LT50ZW1wZXJhdHVyZSA+PSB0cmlwX3RlbXAp
IHt9IGNoZWNrDQo+IGluIHRoZXJtYWxfem9uZV90cmlwX3VwZGF0ZSgpLiINCj4gDQo+IFNvIHdv
dWxkIHNvbWV0aGluZyBsaWtlIHRoZSBmb2xsb3dpbmcgd29yaz8NCj4gDQo+IGRpZmYgLS1naXQg
YS9kcml2ZXJzL3RoZXJtYWwvc3RlcF93aXNlLmMNCj4gYi9kcml2ZXJzL3RoZXJtYWwvc3RlcF93
aXNlLmMNCj4gaW5kZXggZWE5MzY2YS4uMTIyODc5NyAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy90
aGVybWFsL3N0ZXBfd2lzZS5jDQo+ICsrKyBiL2RyaXZlcnMvdGhlcm1hbC9zdGVwX3dpc2UuYw0K
PiBAQCAtMTQzLDcgKzE0Myw3IEBAIHN0YXRpYyB2b2lkIHRoZXJtYWxfem9uZV90cmlwX3VwZGF0
ZShzdHJ1Y3QNCj4gdGhlcm1hbF96b25lX2RldmljZSAqdHosIGludCB0cmlwKQ0KPiDCoMKgDQo+
IMKgwqDCoMKgwqDCoMKgwqDCoHRyZW5kID0gZ2V0X3R6X3RyZW5kKHR6LCB0cmlwKTsNCj4gwqDC
oA0KPiAtwqDCoMKgwqDCoMKgwqBpZiAodHotPnRlbXBlcmF0dXJlID49IHRyaXBfdGVtcCkgew0K
PiArwqDCoMKgwqDCoMKgwqBpZiAodHJpcF90eXBlICE9IFRIRVJNQUxfVFJJUFNfTk9ORSAmJiB0
ei0+dGVtcGVyYXR1cmUgPj0NCj4gdHJpcF90ZW1wKSB7DQoJaWYgKHRyaXBfdGVtcMKgJiYgdHot
PnRlbXBlcmF0dXJlID49IHRyaXBfdGVtcCkgew0KwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqB0
aHJvdHRsZSA9IHRydWU7DQo+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB0cmFj
ZV90aGVybWFsX3pvbmVfdHJpcCh0eiwgdHJpcCwgdHJpcF90eXBlKTsNCj4gwqDCoMKgwqDCoMKg
wqDCoMKgfQ0KPiANCkkgdGhpbmsgUnVpIGlzIHdvcmtpbmcgb24gc29tZSBjaGFuZ2UuDQoNClRo
YW5rcywNClNyaW5pdmFzDQoNCj4gKGNvbXBsZXRlbHkgdW50ZXN0ZWQsIG5vIGlkZWEgaWYgSSdt
IGV2ZW4gY2xvc2UpDQo+IA0KPiBUaGFua3MsDQo+IExhdXJh
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index ea9366a..1228797 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -143,7 +143,7 @@  static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
  
         trend = get_tz_trend(tz, trip);
  
-       if (tz->temperature >= trip_temp) {
+       if (trip_type != THERMAL_TRIPS_NONE && tz->temperature >= trip_temp) {
                 throttle = true;
                 trace_thermal_zone_trip(tz, trip, trip_type);
         }