| Message ID | 1466572546-12944-1-git-send-email-edubezval@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wednesday 22 June 2016 10:45 AM, Eduardo Valentin wrote: > Because several drivers do the following pattern: > .set_mode() > ... > local_data->mode = new_mode; > thermal_zone_device_update(tz); > > makes sense to simply do the thermal_zone_device_update() > in thermal core, after setting the new mode. http://pastebin.ubuntu.com/17687601/ linux-next + current v2 of the patch. The back trace still comes. Regards, Keerthy > > Also, this patch also remove deadlocks on drivers that > call thermal_zone_device_update() on .set_mode(), > as .set_mode() is now called always with tz->lock held. > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Cc: Len Brown <lenb@kernel.org> > Cc: linux-acpi@vger.kernel.org > Cc: "Lee, Chun-Yi" <jlee@suse.com> > Cc: Darren Hart <dvhart@infradead.org> > Cc: Zhang Rui <rui.zhang@intel.com> > Cc: Keerthy <j-keerthy@ti.com> > Cc: linux-kernel@vger.kernel.org > Cc: linux-omap@vger.kernel.org > Cc: platform-driver-x86@vger.kernel.org > Cc: linux-pm@vger.kernel.org > Signed-off-by: Eduardo Valentin <edubezval@gmail.com> > --- > Please ignore last version. > > V1-> V2: > Cleaned the patch and remove unrelated changes. > --- > > drivers/acpi/thermal.c | 2 -- > drivers/platform/x86/acerhdf.c | 1 - > drivers/thermal/imx_thermal.c | 1 - > drivers/thermal/of-thermal.c | 1 - > drivers/thermal/thermal_sysfs.c | 1 + > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 1 - > 6 files changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > index 82707f9..8582b88 100644 > --- a/drivers/acpi/thermal.c > +++ b/drivers/acpi/thermal.c > @@ -519,8 +519,6 @@ static void acpi_thermal_check(void *data) > > if (!tz->tz_enabled) > return; > - > - thermal_zone_device_update(tz->thermal_zone); > } > > /* sys I/F for generic thermal sysfs support */ > diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c > index 460fa67..aee33ba 100644 > --- a/drivers/platform/x86/acerhdf.c > +++ b/drivers/platform/x86/acerhdf.c > @@ -405,7 +405,6 @@ static inline void acerhdf_enable_kernelmode(void) > kernelmode = 1; > > thz_dev->polling_delay = interval*1000; > - thermal_zone_device_update(thz_dev); > pr_notice("kernel mode fan control ON\n"); > } > > diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c > index c5547bd..a413eb6 100644 > --- a/drivers/thermal/imx_thermal.c > +++ b/drivers/thermal/imx_thermal.c > @@ -246,7 +246,6 @@ static int imx_set_mode(struct thermal_zone_device *tz, > } > > data->mode = mode; > - thermal_zone_device_update(tz); > > return 0; > } > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index b8e509c..4602e2e 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -292,7 +292,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz, > mutex_unlock(&tz->lock); > > data->mode = mode; > - thermal_zone_device_update(tz); > > return 0; > } > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > index 743df50..3d0dc30 100644 > --- a/drivers/thermal/thermal_sysfs.c > +++ b/drivers/thermal/thermal_sysfs.c > @@ -100,6 +100,7 @@ mode_store(struct device *dev, struct device_attribute *attr, > mutex_lock(&tz->lock); > result = tz->ops->set_mode(tz, mode); > mutex_unlock(&tz->lock); > + thermal_zone_device_update(tz); > > if (result) > return result; > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > index 15c0a9a..9a5a3a3 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > @@ -205,7 +205,6 @@ static int ti_thermal_set_mode(struct thermal_zone_device *thermal, > data->mode = mode; > ti_bandgap_write_update_interval(bgp, data->sensor_id, > data->ti_thermal->polling_delay); > - thermal_zone_device_update(data->ti_thermal); > dev_dbg(&thermal->device, "thermal polling set for duration=%d msec\n", > data->ti_thermal->polling_delay); > > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 22, 2016 at 03:03:45PM +0530, Keerthy wrote: > > > On Wednesday 22 June 2016 10:45 AM, Eduardo Valentin wrote: > >Because several drivers do the following pattern: > >.set_mode() > > ... > > local_data->mode = new_mode; > > thermal_zone_device_update(tz); > > > >makes sense to simply do the thermal_zone_device_update() > >in thermal core, after setting the new mode. > > http://pastebin.ubuntu.com/17687601/ > > linux-next + current v2 of the patch. > The back trace still comes. I confirm this. Please check V3 I just sent. On V2 I focused on thermal_zone_device_update(), but for OF thermal specifically, we also hold the lock for protecting other accesses. Now they are removed. > > Regards, > Keerthy > > > > >Also, this patch also remove deadlocks on drivers that > >call thermal_zone_device_update() on .set_mode(), > >as .set_mode() is now called always with tz->lock held. > > > >Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > >Cc: Len Brown <lenb@kernel.org> > >Cc: linux-acpi@vger.kernel.org > >Cc: "Lee, Chun-Yi" <jlee@suse.com> > >Cc: Darren Hart <dvhart@infradead.org> > >Cc: Zhang Rui <rui.zhang@intel.com> > >Cc: Keerthy <j-keerthy@ti.com> > >Cc: linux-kernel@vger.kernel.org > >Cc: linux-omap@vger.kernel.org > >Cc: platform-driver-x86@vger.kernel.org > >Cc: linux-pm@vger.kernel.org > >Signed-off-by: Eduardo Valentin <edubezval@gmail.com> > >--- > >Please ignore last version. > > > >V1-> V2: > >Cleaned the patch and remove unrelated changes. > >--- > > > > drivers/acpi/thermal.c | 2 -- > > drivers/platform/x86/acerhdf.c | 1 - > > drivers/thermal/imx_thermal.c | 1 - > > drivers/thermal/of-thermal.c | 1 - > > drivers/thermal/thermal_sysfs.c | 1 + > > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 1 - > > 6 files changed, 1 insertion(+), 6 deletions(-) > > > >diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > >index 82707f9..8582b88 100644 > >--- a/drivers/acpi/thermal.c > >+++ b/drivers/acpi/thermal.c > >@@ -519,8 +519,6 @@ static void acpi_thermal_check(void *data) > > > > if (!tz->tz_enabled) > > return; > >- > >- thermal_zone_device_update(tz->thermal_zone); > > } > > > > /* sys I/F for generic thermal sysfs support */ > >diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c > >index 460fa67..aee33ba 100644 > >--- a/drivers/platform/x86/acerhdf.c > >+++ b/drivers/platform/x86/acerhdf.c > >@@ -405,7 +405,6 @@ static inline void acerhdf_enable_kernelmode(void) > > kernelmode = 1; > > > > thz_dev->polling_delay = interval*1000; > >- thermal_zone_device_update(thz_dev); > > pr_notice("kernel mode fan control ON\n"); > > } > > > >diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c > >index c5547bd..a413eb6 100644 > >--- a/drivers/thermal/imx_thermal.c > >+++ b/drivers/thermal/imx_thermal.c > >@@ -246,7 +246,6 @@ static int imx_set_mode(struct thermal_zone_device *tz, > > } > > > > data->mode = mode; > >- thermal_zone_device_update(tz); > > > > return 0; > > } > >diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > >index b8e509c..4602e2e 100644 > >--- a/drivers/thermal/of-thermal.c > >+++ b/drivers/thermal/of-thermal.c > >@@ -292,7 +292,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz, > > mutex_unlock(&tz->lock); > > > > data->mode = mode; > >- thermal_zone_device_update(tz); > > > > return 0; > > } > >diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > >index 743df50..3d0dc30 100644 > >--- a/drivers/thermal/thermal_sysfs.c > >+++ b/drivers/thermal/thermal_sysfs.c > >@@ -100,6 +100,7 @@ mode_store(struct device *dev, struct device_attribute *attr, > > mutex_lock(&tz->lock); > > result = tz->ops->set_mode(tz, mode); > > mutex_unlock(&tz->lock); > >+ thermal_zone_device_update(tz); > > > > if (result) > > return result; > >diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > >index 15c0a9a..9a5a3a3 100644 > >--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > >+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > >@@ -205,7 +205,6 @@ static int ti_thermal_set_mode(struct thermal_zone_device *thermal, > > data->mode = mode; > > ti_bandgap_write_update_interval(bgp, data->sensor_id, > > data->ti_thermal->polling_delay); > >- thermal_zone_device_update(data->ti_thermal); > > dev_dbg(&thermal->device, "thermal polling set for duration=%d msec\n", > > data->ti_thermal->polling_delay); > > > > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 22, 2016 at 07:36:11AM -0700, Eduardo Valentin wrote: > On Wed, Jun 22, 2016 at 03:03:45PM +0530, Keerthy wrote: > > > > > > On Wednesday 22 June 2016 10:45 AM, Eduardo Valentin wrote: > > >Because several drivers do the following pattern: > > >.set_mode() > > > ... > > > local_data->mode = new_mode; > > > thermal_zone_device_update(tz); > > > > > >makes sense to simply do the thermal_zone_device_update() > > >in thermal core, after setting the new mode. > > > > http://pastebin.ubuntu.com/17687601/ > > > > linux-next + current v2 of the patch. > > The back trace still comes. > > I confirm this. Please check V3 I just sent. On V2 I focused > on thermal_zone_device_update(), but for OF thermal specifically, > we also hold the lock for protecting other accesses. Now they > are removed. Please use v4. I removed extra non needed locking in ti SoC driver for case which boots without DT (not sure if this is still used though). Also, improved acpi thermal, as it was remove the update from other needed cases. > > > > > Regards, > > Keerthy > > > > > > > >Also, this patch also remove deadlocks on drivers that > > >call thermal_zone_device_update() on .set_mode(), > > >as .set_mode() is now called always with tz->lock held. > > > > > >Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > > >Cc: Len Brown <lenb@kernel.org> > > >Cc: linux-acpi@vger.kernel.org > > >Cc: "Lee, Chun-Yi" <jlee@suse.com> > > >Cc: Darren Hart <dvhart@infradead.org> > > >Cc: Zhang Rui <rui.zhang@intel.com> > > >Cc: Keerthy <j-keerthy@ti.com> > > >Cc: linux-kernel@vger.kernel.org > > >Cc: linux-omap@vger.kernel.org > > >Cc: platform-driver-x86@vger.kernel.org > > >Cc: linux-pm@vger.kernel.org > > >Signed-off-by: Eduardo Valentin <edubezval@gmail.com> > > >--- > > >Please ignore last version. > > > > > >V1-> V2: > > >Cleaned the patch and remove unrelated changes. > > >--- > > > > > > drivers/acpi/thermal.c | 2 -- > > > drivers/platform/x86/acerhdf.c | 1 - > > > drivers/thermal/imx_thermal.c | 1 - > > > drivers/thermal/of-thermal.c | 1 - > > > drivers/thermal/thermal_sysfs.c | 1 + > > > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 1 - > > > 6 files changed, 1 insertion(+), 6 deletions(-) > > > > > >diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > > >index 82707f9..8582b88 100644 > > >--- a/drivers/acpi/thermal.c > > >+++ b/drivers/acpi/thermal.c > > >@@ -519,8 +519,6 @@ static void acpi_thermal_check(void *data) > > > > > > if (!tz->tz_enabled) > > > return; > > >- > > >- thermal_zone_device_update(tz->thermal_zone); > > > } > > > > > > /* sys I/F for generic thermal sysfs support */ > > >diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c > > >index 460fa67..aee33ba 100644 > > >--- a/drivers/platform/x86/acerhdf.c > > >+++ b/drivers/platform/x86/acerhdf.c > > >@@ -405,7 +405,6 @@ static inline void acerhdf_enable_kernelmode(void) > > > kernelmode = 1; > > > > > > thz_dev->polling_delay = interval*1000; > > >- thermal_zone_device_update(thz_dev); > > > pr_notice("kernel mode fan control ON\n"); > > > } > > > > > >diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c > > >index c5547bd..a413eb6 100644 > > >--- a/drivers/thermal/imx_thermal.c > > >+++ b/drivers/thermal/imx_thermal.c > > >@@ -246,7 +246,6 @@ static int imx_set_mode(struct thermal_zone_device *tz, > > > } > > > > > > data->mode = mode; > > >- thermal_zone_device_update(tz); > > > > > > return 0; > > > } > > >diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > > >index b8e509c..4602e2e 100644 > > >--- a/drivers/thermal/of-thermal.c > > >+++ b/drivers/thermal/of-thermal.c > > >@@ -292,7 +292,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz, > > > mutex_unlock(&tz->lock); > > > > > > data->mode = mode; > > >- thermal_zone_device_update(tz); > > > > > > return 0; > > > } > > >diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > > >index 743df50..3d0dc30 100644 > > >--- a/drivers/thermal/thermal_sysfs.c > > >+++ b/drivers/thermal/thermal_sysfs.c > > >@@ -100,6 +100,7 @@ mode_store(struct device *dev, struct device_attribute *attr, > > > mutex_lock(&tz->lock); > > > result = tz->ops->set_mode(tz, mode); > > > mutex_unlock(&tz->lock); > > >+ thermal_zone_device_update(tz); > > > > > > if (result) > > > return result; > > >diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > >index 15c0a9a..9a5a3a3 100644 > > >--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > >+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > >@@ -205,7 +205,6 @@ static int ti_thermal_set_mode(struct thermal_zone_device *thermal, > > > data->mode = mode; > > > ti_bandgap_write_update_interval(bgp, data->sensor_id, > > > data->ti_thermal->polling_delay); > > >- thermal_zone_device_update(data->ti_thermal); > > > dev_dbg(&thermal->device, "thermal polling set for duration=%d msec\n", > > > data->ti_thermal->polling_delay); > > > > > > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index 82707f9..8582b88 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -519,8 +519,6 @@ static void acpi_thermal_check(void *data) if (!tz->tz_enabled) return; - - thermal_zone_device_update(tz->thermal_zone); } /* sys I/F for generic thermal sysfs support */ diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c index 460fa67..aee33ba 100644 --- a/drivers/platform/x86/acerhdf.c +++ b/drivers/platform/x86/acerhdf.c @@ -405,7 +405,6 @@ static inline void acerhdf_enable_kernelmode(void) kernelmode = 1; thz_dev->polling_delay = interval*1000; - thermal_zone_device_update(thz_dev); pr_notice("kernel mode fan control ON\n"); } diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c index c5547bd..a413eb6 100644 --- a/drivers/thermal/imx_thermal.c +++ b/drivers/thermal/imx_thermal.c @@ -246,7 +246,6 @@ static int imx_set_mode(struct thermal_zone_device *tz, } data->mode = mode; - thermal_zone_device_update(tz); return 0; } diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index b8e509c..4602e2e 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -292,7 +292,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz, mutex_unlock(&tz->lock); data->mode = mode; - thermal_zone_device_update(tz); return 0; } diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index 743df50..3d0dc30 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -100,6 +100,7 @@ mode_store(struct device *dev, struct device_attribute *attr, mutex_lock(&tz->lock); result = tz->ops->set_mode(tz, mode); mutex_unlock(&tz->lock); + thermal_zone_device_update(tz); if (result) return result; diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c index 15c0a9a..9a5a3a3 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -205,7 +205,6 @@ static int ti_thermal_set_mode(struct thermal_zone_device *thermal, data->mode = mode; ti_bandgap_write_update_interval(bgp, data->sensor_id, data->ti_thermal->polling_delay); - thermal_zone_device_update(data->ti_thermal); dev_dbg(&thermal->device, "thermal polling set for duration=%d msec\n", data->ti_thermal->polling_delay);
Because several drivers do the following pattern: .set_mode() ... local_data->mode = new_mode; thermal_zone_device_update(tz); makes sense to simply do the thermal_zone_device_update() in thermal core, after setting the new mode. Also, this patch also remove deadlocks on drivers that call thermal_zone_device_update() on .set_mode(), as .set_mode() is now called always with tz->lock held. Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Len Brown <lenb@kernel.org> Cc: linux-acpi@vger.kernel.org Cc: "Lee, Chun-Yi" <jlee@suse.com> Cc: Darren Hart <dvhart@infradead.org> Cc: Zhang Rui <rui.zhang@intel.com> Cc: Keerthy <j-keerthy@ti.com> Cc: linux-kernel@vger.kernel.org Cc: linux-omap@vger.kernel.org Cc: platform-driver-x86@vger.kernel.org Cc: linux-pm@vger.kernel.org Signed-off-by: Eduardo Valentin <edubezval@gmail.com> --- Please ignore last version. V1-> V2: Cleaned the patch and remove unrelated changes. --- drivers/acpi/thermal.c | 2 -- drivers/platform/x86/acerhdf.c | 1 - drivers/thermal/imx_thermal.c | 1 - drivers/thermal/of-thermal.c | 1 - drivers/thermal/thermal_sysfs.c | 1 + drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 1 - 6 files changed, 1 insertion(+), 6 deletions(-)