mbox series

[0/4] thermal: fix locking regressions in linux-next

Message ID 20221214131617.2447-1-johan+linaro@kernel.org (mailing list archive)
Headers show
Series thermal: fix locking regressions in linux-next | expand

Message

Johan Hovold Dec. 14, 2022, 1:16 p.m. UTC
This series fixes some of the fallout after the thermal changes that
just landed in linux-next.

Lockdep reported a lock inversion in one of the Qualcomm drivers and a
closer review revealed that the changes had also broken the sysfs
interface for at least three drivers.

Note that a simple revert of the offending patches was not an option as
some of the infrastructure that the old implementation relied on has
also been removed.

Johan


Johan Hovold (4):
  thermal/drivers/qcom: fix set_trip_temp() deadlock
  thermal/drivers/exynos: fix set_trip_temp() deadlock
  thermal/drivers/tegra: fix set_trip_temp() deadlock
  thermal/drivers/qcom: fix lock inversion

 drivers/thermal/qcom/qcom-spmi-temp-alarm.c | 7 ++++++-
 drivers/thermal/samsung/exynos_tmu.c        | 2 +-
 drivers/thermal/tegra/soctherm.c            | 2 +-
 drivers/thermal/thermal_core.c              | 1 +
 include/linux/thermal.h                     | 2 ++
 5 files changed, 11 insertions(+), 3 deletions(-)

Comments

Rafael J. Wysocki Dec. 14, 2022, 2:02 p.m. UTC | #1
On Wed, Dec 14, 2022 at 2:18 PM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> This series fixes some of the fallout after the thermal changes that
> just landed in linux-next.
>
> Lockdep reported a lock inversion in one of the Qualcomm drivers and a
> closer review revealed that the changes had also broken the sysfs
> interface for at least three drivers.
>
> Note that a simple revert of the offending patches was not an option as
> some of the infrastructure that the old implementation relied on has
> also been removed.

I've dropped that material from my linux-next branch and Daniel,
please also remove it from your branch that is pulled by linux-next so
that it doesn't show up in there until 6.2-rc1 is out.

It clearly is not ready for merging in its current form.

It is still present in my bleeding-edge branch, though, so please
apply the patches from Johan on top of it and send a new PR to me, so
I can add it back to my linux-next branch once 6.2-rc1 appears.

It would be good to check the code again too for any more similar fallout.

Thanks!
Daniel Lezcano Dec. 14, 2022, 2:37 p.m. UTC | #2
On 14/12/2022 15:02, Rafael J. Wysocki wrote:
> On Wed, Dec 14, 2022 at 2:18 PM Johan Hovold <johan+linaro@kernel.org> wrote:
>>
>> This series fixes some of the fallout after the thermal changes that
>> just landed in linux-next.
>>
>> Lockdep reported a lock inversion in one of the Qualcomm drivers and a
>> closer review revealed that the changes had also broken the sysfs
>> interface for at least three drivers.
>>
>> Note that a simple revert of the offending patches was not an option as
>> some of the infrastructure that the old implementation relied on has
>> also been removed.
> 
> I've dropped that material from my linux-next branch and Daniel,
> please also remove it from your branch that is pulled by linux-next so
> that it doesn't show up in there until 6.2-rc1 is out.
> 
> It clearly is not ready for merging in its current form.

I rebased a linux-next branch without the generic trip points rework.

It can be inverted with the other changes without conflicts.

I've pushed the branch in case you want to have a look. If you think it 
is acceptable in this form, I can send a tagged PR for 6.2-rc1 again.

> It is still present in my bleeding-edge branch, though, so please
> apply the patches from Johan on top of it and send a new PR to me, so
> I can add it back to my linux-next branch once 6.2-rc1 appears.
> 
> It would be good to check the code again too for any more similar fallout.

I've been through already, the exynos fix is not necessary. But anyway, 
I agree we should keep these changes for the next release, it is better.
Johan Hovold Dec. 14, 2022, 2:43 p.m. UTC | #3
On Wed, Dec 14, 2022 at 03:37:43PM +0100, Daniel Lezcano wrote:
> On 14/12/2022 15:02, Rafael J. Wysocki wrote:
> > On Wed, Dec 14, 2022 at 2:18 PM Johan Hovold <johan+linaro@kernel.org> wrote:
> >>
> >> This series fixes some of the fallout after the thermal changes that
> >> just landed in linux-next.
> >>
> >> Lockdep reported a lock inversion in one of the Qualcomm drivers and a
> >> closer review revealed that the changes had also broken the sysfs
> >> interface for at least three drivers.

> > It is still present in my bleeding-edge branch, though, so please
> > apply the patches from Johan on top of it and send a new PR to me, so
> > I can add it back to my linux-next branch once 6.2-rc1 appears.
> > 
> > It would be good to check the code again too for any more similar fallout.
> 
> I've been through already, the exynos fix is not necessary.

Right, I failed to notice that tmu_set_trip_temp() was not actually a
thermal_zone_device_ops callback. So that one can be dropped.

Johan
Rafael J. Wysocki Dec. 14, 2022, 2:46 p.m. UTC | #4
On Wed, Dec 14, 2022 at 3:37 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 14/12/2022 15:02, Rafael J. Wysocki wrote:
> > On Wed, Dec 14, 2022 at 2:18 PM Johan Hovold <johan+linaro@kernel.org> wrote:
> >>
> >> This series fixes some of the fallout after the thermal changes that
> >> just landed in linux-next.
> >>
> >> Lockdep reported a lock inversion in one of the Qualcomm drivers and a
> >> closer review revealed that the changes had also broken the sysfs
> >> interface for at least three drivers.
> >>
> >> Note that a simple revert of the offending patches was not an option as
> >> some of the infrastructure that the old implementation relied on has
> >> also been removed.
> >
> > I've dropped that material from my linux-next branch and Daniel,
> > please also remove it from your branch that is pulled by linux-next so
> > that it doesn't show up in there until 6.2-rc1 is out.
> >
> > It clearly is not ready for merging in its current form.
>
> I rebased a linux-next branch without the generic trip points rework.

Thanks!

> It can be inverted with the other changes without conflicts.

Sounds good.

> I've pushed the branch in case you want to have a look. If you think it
> is acceptable in this form, I can send a tagged PR for 6.2-rc1 again.

It looks OK, so please send a PR.

> > It is still present in my bleeding-edge branch, though, so please
> > apply the patches from Johan on top of it and send a new PR to me, so
> > I can add it back to my linux-next branch once 6.2-rc1 appears.
> >
> > It would be good to check the code again too for any more similar fallout.
>
> I've been through already, the exynos fix is not necessary. But anyway,
> I agree we should keep these changes for the next release, it is better.

Awesome, thanks!
Daniel Lezcano Dec. 14, 2022, 2:51 p.m. UTC | #5
Hi Johan,

thanks for your fixes

On 14/12/2022 14:16, Johan Hovold wrote:
> This series fixes some of the fallout after the thermal changes that
> just landed in linux-next.
> 
> Lockdep reported a lock inversion in one of the Qualcomm drivers and a
> closer review revealed that the changes had also broken the sysfs
> interface for at least three drivers.
> 
> Note that a simple revert of the offending patches was not an option as
> some of the infrastructure that the old implementation relied on has
> also been removed.
> 
> Johan
> 
> 
> Johan Hovold (4):
>    thermal/drivers/qcom: fix set_trip_temp() deadlock
>    thermal/drivers/exynos: fix set_trip_temp() deadlock
>    thermal/drivers/tegra: fix set_trip_temp() deadlock
>    thermal/drivers/qcom: fix lock inversion
> 
>   drivers/thermal/qcom/qcom-spmi-temp-alarm.c | 7 ++++++-
>   drivers/thermal/samsung/exynos_tmu.c        | 2 +-
>   drivers/thermal/tegra/soctherm.c            | 2 +-
>   drivers/thermal/thermal_core.c              | 1 +
>   include/linux/thermal.h                     | 2 ++
>   5 files changed, 11 insertions(+), 3 deletions(-)
>