lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 07 Oct 2014 22:05:28 +0900 From: jonghwa3.lee@...sung.com To: Krzysztof Kozlowski <k.kozlowski@...sung.com> Cc: Sebastian Reichel <sre@...nel.org>, Dmitry Eremin-Solenikov <dbaryshkov@...il.com>, David Woodhouse <dwmw2@...radead.org>, Myungjoo Ham <myungjoo.ham@...sung.com>, Anton Vorontsov <anton@...msg.org>, Chanwoo Choi <cw00.choi@...sung.com>, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] power: charger-manager: Avoid recursive thermal get_temp call On 2014년 10월 07일 21:50, Krzysztof Kozlowski wrote: > On wto, 2014-10-07 at 19:18 +0900, jonghwa3.lee@...sung.com wrote: >> On 2014년 10월 07일 16:13, Krzysztof Kozlowski wrote: >> >>> >>> On wto, 2014-10-07 at 11:20 +0900, jonghwa3.lee@...sung.com wrote: >>>> Hi, >>>> On 2014년 10월 07일 01:00, Krzysztof Kozlowski wrote: >>>> >>>>> The charger manager supports POWER_SUPPLY_PROP_TEMP property and acts >>>>> as a thermal zone if any of these conditions match: >>>>> 1. Fuel gauge used by charger manager supports POWER_SUPPLY_PROP_TEMP. >>>>> 2. 'cm-thermal-zone' property is present in DTS (then it will supersede >>>>> the fuel gauge temperature property). >>>>> >>>>> However in case 1 (fuel gauge reports temperature and 'cm-thermal-zone' >>>>> is not set) the charger manager forwards its get_temp calls to fuel >>>>> gauge thermal zone. >>>>> >>>>> This leads to reporting by lockdep a false positive deadlock for thermal >>>>> zone's mutex because of nested calls to thermal_zone_get_temp(). This is >>>>> false positive because these are different mutexes: one for charger >>>>> manager thermal zone and second for fuel gauge thermal zone. >>>>> >>>> >>>> >>>> Yes, this happens because power_supply_subsystem automatically creates >>>> thermal_zone when POWER_SUPPLY_PROP_TEMP is available at the time of >>>> power_supply registering. And as you point out, it makes duplicate thermal_zone >>>> when some power_supply references other power_supply's. I hope it would become >>>> to be a selectable option or change the manner of charger-manager itself (if the >>>> charger-manager is only one who references other power_supply's temperature >>>> sensing ability). >>>> >>>> Anyway, the code looks fine to me. >>>> >>>> Acked-by : Jonghwa Lee <jonghwa3.lee@...sung.com> >>> >>> Thank you for looking at patch. However later I started to wonder >>> whether my fix is sufficient. For the case when fuel gauge is used as >>> source of temperature it is. But for the case when external sensor is >>> used it is not - still there will be recursive call and false positive >>> from lockdep. >>> >>> Also minor fix is needed in my patch - s/IS_ENABLED/config_enabled/. >>> >>> I can send a v2 fixing this but first question - what about second >>> recursive issue (when external sensor is used instead of fuel gauge)? >>> >> >> >> Yes, you're right, it still had problem for external temperature sensor case. >> How about we change the core not to make duplicate thermal zone if it already >> existed. It's not the common case, but it looks worthy. >> >> like as below, >> >> static int psy_register_thermal(struct power_supply *psy) >> { >> ... >> + if (psy->tzd) >> + return 0; >> ... >> >> We also needs some modification at charger-manager driver, however we can avoid >> recursive call problem with this way :) > > Yes, that would remove recursive call but this would also remove thermal > zone for charger manager's power supply. Does anyone (user space?) > depend and use charger managers thermal zone? > AFAIK, no,, charger manager's thermal zone means nothing but just replica of other thermal zone related with battery. It is better to remove it. Thanks, Jonghwa. > Best regards, > Krzysztof > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists