Hi Everyone,
This series changes the PM core to copy the trips and zone ops directly
into struct thermal_zone_device so as to allow the callers of the zone
registration function to discard their own copies of those things after
zone registration and/or possibly allocate them as read-only.
The first patch makes the thermal core create a copy of the trips table which
is declared as a flex array to enable additional bounds checking on it. The
next two patches update the ACPI thermal driver and Intel thermal drivers to
take benefit of that change.
In a similar pattern, patch [4/6] makes the thermal core create an internal
copy of the zone ops supplied by the zone creator, so as to allow the
original ops structure to be discarded after zone registration or allocated
as read-only, and the next two patches update the ACPI thermal driver and Intel
thermal drivers to actually do that.
The other thermal drivers need not be changed, although in principle they may
be simplified a bit too in the future.
As usual, please refer to the individual patch changelogs for details.
Thanks!
From: Rafael J. Wysocki <[email protected]>
The current code requires thermal zone creators to pass a pointer to a
writable trips table to thermal_zone_device_register_with_trips() and
that trips table is then used by the thermal core going forward.
Consequently, the callers of thermal_zone_device_register_with_trips()
are required to hold on to the trips table passed to it until the given
thermal zone is unregistered, at which point the trips table can be
freed, but at the same time they are not allowed to access the cells in
that table directly. This is both error prone and confusing.
To address it, turn the trips table pointer in struct thermal_zone_device
into a flex array (counted by its num_trips field), allocate it during
thermal zone device allocation and copy the contents of the trips table
supplied by the zone creator (which can be const now) into it.
This allows the callers of thermal_zone_device_register_with_trips() to
drop their trip tables right after the zone registration.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/thermal_core.c | 16 +++++++++-------
include/linux/thermal.h | 10 +++++-----
2 files changed, 14 insertions(+), 12 deletions(-)
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -130,7 +130,6 @@ struct thermal_cooling_device {
* @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
* @mode: current mode of this thermal zone
* @devdata: private pointer for device private data
- * @trips: an array of struct thermal_trip
* @num_trips: number of trip points the thermal zone supports
* @passive_delay_jiffies: number of jiffies to wait between polls when
* performing passive cooling.
@@ -160,6 +159,7 @@ struct thermal_cooling_device {
* @poll_queue: delayed work for polling
* @notify_event: Last notification event
* @suspended: thermal zone suspend indicator
+ * @trips: array of struct thermal_trip objects
*/
struct thermal_zone_device {
int id;
@@ -172,7 +172,6 @@ struct thermal_zone_device {
struct thermal_attr *trip_hyst_attrs;
enum thermal_device_mode mode;
void *devdata;
- struct thermal_trip *trips;
int num_trips;
unsigned long passive_delay_jiffies;
unsigned long polling_delay_jiffies;
@@ -193,10 +192,11 @@ struct thermal_zone_device {
struct list_head node;
struct delayed_work poll_queue;
enum thermal_notify_event notify_event;
+ bool suspended;
#ifdef CONFIG_THERMAL_DEBUGFS
struct thermal_debugfs *debugfs;
#endif
- bool suspended;
+ struct thermal_trip trips[] __counted_by(num_trips);
};
/**
@@ -315,7 +315,7 @@ int thermal_zone_get_crit_temp(struct th
#ifdef CONFIG_THERMAL
struct thermal_zone_device *thermal_zone_device_register_with_trips(
const char *type,
- struct thermal_trip *trips,
+ const struct thermal_trip *trips,
int num_trips, int mask,
void *devdata,
struct thermal_zone_device_ops *ops,
@@ -375,7 +375,7 @@ void thermal_zone_device_critical(struct
#else
static inline struct thermal_zone_device *thermal_zone_device_register_with_trips(
const char *type,
- struct thermal_trip *trips,
+ const struct thermal_trip *trips,
int num_trips, int mask,
void *devdata,
struct thermal_zone_device_ops *ops,
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1272,10 +1272,13 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
* IS_ERR*() helpers.
*/
struct thermal_zone_device *
-thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int num_trips, int mask,
- void *devdata, struct thermal_zone_device_ops *ops,
- const struct thermal_zone_params *tzp, int passive_delay,
- int polling_delay)
+thermal_zone_device_register_with_trips(const char *type,
+ const struct thermal_trip *trips,
+ int num_trips, int mask,
+ void *devdata,
+ struct thermal_zone_device_ops *ops,
+ const struct thermal_zone_params *tzp,
+ int passive_delay, int polling_delay)
{
struct thermal_zone_device *tz;
int id;
@@ -1322,7 +1325,7 @@ thermal_zone_device_register_with_trips(
if (!thermal_class)
return ERR_PTR(-ENODEV);
- tz = kzalloc(sizeof(*tz), GFP_KERNEL);
+ tz = kzalloc(struct_size(tz, trips, num_trips), GFP_KERNEL);
if (!tz)
return ERR_PTR(-ENOMEM);
@@ -1344,7 +1347,6 @@ thermal_zone_device_register_with_trips(
result = id;
goto free_tzp;
}
-
tz->id = id;
strscpy(tz->type, type, sizeof(tz->type));
@@ -1354,7 +1356,7 @@ thermal_zone_device_register_with_trips(
tz->ops = ops;
tz->device.class = thermal_class;
tz->devdata = devdata;
- tz->trips = trips;
+ memcpy(tz->trips, trips, num_trips * sizeof(trips[0]));
tz->num_trips = num_trips;
thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
From: Rafael J. Wysocki <[email protected]>
Because the thermal core creates and uses its own copy of the trips
table passed to thermal_zone_device_register_with_trips(), it is not
necessary to hold on to a local copy of it any more after the given
thermal zone has been registered.
Accordingly, modify Intel thermal drivers to discard the trips tables
passed to thermal_zone_device_register_with_trips() after thermal zone
registration, for example by storing them in local variables which are
automatically discarded when the zone registration is complete.
Also make some additional code simplifications unlocked by the above
changes.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 6 -
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h | 1
drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c | 6 -
drivers/thermal/intel/intel_pch_thermal.c | 21 ++---
drivers/thermal/intel/intel_quark_dts_thermal.c | 12 +--
drivers/thermal/intel/intel_soc_dts_iosf.c | 38 +++-------
drivers/thermal/intel/intel_soc_dts_iosf.h | 1
drivers/thermal/intel/x86_pkg_temp_thermal.c | 35 +++------
8 files changed, 48 insertions(+), 72 deletions(-)
Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -179,8 +179,6 @@ struct int34x_thermal_zone *int340x_ther
for (i = 0; i < trip_cnt; ++i)
zone_trips[i].hysteresis = hyst;
- int34x_zone->trips = zone_trips;
-
int34x_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
int34x_zone->zone = thermal_zone_device_register_with_trips(
@@ -190,6 +188,8 @@ struct int34x_thermal_zone *int340x_ther
int34x_zone->ops,
&int340x_thermal_params,
0, 0);
+ kfree(zone_trips);
+
if (IS_ERR(int34x_zone->zone)) {
ret = PTR_ERR(int34x_zone->zone);
goto err_thermal_zone;
@@ -203,7 +203,6 @@ struct int34x_thermal_zone *int340x_ther
err_enable:
thermal_zone_device_unregister(int34x_zone->zone);
err_thermal_zone:
- kfree(int34x_zone->trips);
acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
err_trips_alloc:
kfree(int34x_zone->ops);
@@ -217,7 +216,6 @@ void int340x_thermal_zone_remove(struct
{
thermal_zone_device_unregister(int34x_zone->zone);
acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
- kfree(int34x_zone->trips);
kfree(int34x_zone->ops);
kfree(int34x_zone);
}
Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
@@ -20,7 +20,6 @@ struct active_trip {
struct int34x_thermal_zone {
struct acpi_device *adev;
- struct thermal_trip *trips;
int aux_trip_nr;
struct thermal_zone_device *zone;
struct thermal_zone_device_ops *ops;
Index: linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
@@ -233,10 +233,6 @@ static int get_trip_temp(struct proc_the
return temp;
}
-static struct thermal_trip psv_trip = {
- .type = THERMAL_TRIP_PASSIVE,
-};
-
static struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
.set_trip_temp = sys_set_trip_temp,
@@ -251,6 +247,7 @@ static int proc_thermal_pci_probe(struct
{
struct proc_thermal_device *proc_priv;
struct proc_thermal_pci *pci_info;
+ struct thermal_trip psv_trip = { 0 };
int irq_flag = 0, irq, ret;
bool msi_irq = false;
@@ -288,6 +285,7 @@ static int proc_thermal_pci_probe(struct
}
psv_trip.temperature = get_trip_temp(pci_info);
+ psv_trip.type = THERMAL_TRIP_PASSIVE;
pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
1, 1, pci_info,
Index: linux-pm/drivers/thermal/intel/intel_pch_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_pch_thermal.c
+++ linux-pm/drivers/thermal/intel/intel_pch_thermal.c
@@ -84,7 +84,6 @@ struct pch_thermal_device {
void __iomem *hw_base;
struct pci_dev *pdev;
struct thermal_zone_device *tzd;
- struct thermal_trip trips[PCH_MAX_TRIPS];
bool bios_enabled;
};
@@ -94,7 +93,8 @@ struct pch_thermal_device {
* passive trip temperature using _PSV method. There is no specific
* passive temperature setting in MMIO interface of this PCI device.
*/
-static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd, int trip)
+static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd,
+ struct thermal_trip *trip)
{
struct acpi_device *adev;
int temp;
@@ -106,8 +106,8 @@ static int pch_wpt_add_acpi_psv_trip(str
if (thermal_acpi_passive_trip_temp(adev, &temp) || temp <= 0)
return 0;
- ptd->trips[trip].type = THERMAL_TRIP_PASSIVE;
- ptd->trips[trip].temperature = temp;
+ trip->type = THERMAL_TRIP_PASSIVE;
+ trip->temperature = temp;
return 1;
}
#else
@@ -159,6 +159,7 @@ static const char *board_names[] = {
static int intel_pch_thermal_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
+ struct thermal_trip ptd_trips[PCH_MAX_TRIPS] = { 0 };
enum pch_board_ids board_id = id->driver_data;
struct pch_thermal_device *ptd;
int nr_trips = 0;
@@ -220,21 +221,21 @@ read_trips:
trip_temp = readw(ptd->hw_base + WPT_CTT);
trip_temp &= 0x1FF;
if (trip_temp) {
- ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
- ptd->trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
+ ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
+ ptd_trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
}
trip_temp = readw(ptd->hw_base + WPT_PHL);
trip_temp &= 0x1FF;
if (trip_temp) {
- ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
- ptd->trips[nr_trips++].type = THERMAL_TRIP_HOT;
+ ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
+ ptd_trips[nr_trips++].type = THERMAL_TRIP_HOT;
}
- nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
+ nr_trips += pch_wpt_add_acpi_psv_trip(ptd, &ptd_trips[nr_trips]);
ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
- ptd->trips, nr_trips,
+ ptd_trips, nr_trips,
0, ptd, &tzd_ops,
NULL, 0, 0);
if (IS_ERR(ptd->tzd)) {
Index: linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -105,7 +105,6 @@ struct soc_sensor_entry {
u32 store_ptps;
u32 store_dts_enable;
struct thermal_zone_device *tzone;
- struct thermal_trip trips[QRK_MAX_DTS_TRIPS];
};
static struct soc_sensor_entry *soc_dts;
@@ -320,6 +319,7 @@ static void free_soc_dts(struct soc_sens
static struct soc_sensor_entry *alloc_soc_dts(void)
{
+ struct thermal_trip trips[QRK_MAX_DTS_TRIPS] = { 0 };
struct soc_sensor_entry *aux_entry;
int err;
u32 out;
@@ -362,14 +362,14 @@ static struct soc_sensor_entry *alloc_so
goto err_ret;
}
- aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
- aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
+ trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
+ trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
- aux_entry->trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
- aux_entry->trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
+ trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
+ trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
aux_entry->tzone = thermal_zone_device_register_with_trips("quark_dts",
- aux_entry->trips,
+ trips,
QRK_MAX_DTS_TRIPS,
wr_mask,
aux_entry, &tzone_ops,
Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
+++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
@@ -129,22 +129,6 @@ err_restore_ptps:
return status;
}
-static int configure_trip(struct intel_soc_dts_sensor_entry *dts,
- int thres_index, enum thermal_trip_type trip_type,
- int temp)
-{
- int ret;
-
- ret = update_trip_temp(dts->sensors, thres_index, temp);
- if (ret)
- return ret;
-
- dts->trips[thres_index].temperature = temp;
- dts->trips[thres_index].type = trip_type;
-
- return 0;
-}
-
static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
int temp)
{
@@ -218,6 +202,7 @@ static void remove_dts_thermal_zone(stru
}
static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
+ struct thermal_trip *trips,
bool critical_trip)
{
int writable_trip_cnt = SOC_MAX_DTS_TRIPS;
@@ -254,7 +239,7 @@ static int add_dts_thermal_zone(int id,
}
dts->trip_mask = trip_mask;
snprintf(name, sizeof(name), "soc_dts%d", id);
- dts->tzone = thermal_zone_device_register_with_trips(name, dts->trips,
+ dts->tzone = thermal_zone_device_register_with_trips(name, trips,
SOC_MAX_DTS_TRIPS,
trip_mask,
dts, &tzone_ops,
@@ -315,14 +300,15 @@ EXPORT_SYMBOL_GPL(intel_soc_dts_iosf_int
static void dts_trips_reset(struct intel_soc_dts_sensors *sensors, int dts_index)
{
- configure_trip(&sensors->soc_dts[dts_index], 0, 0, 0);
- configure_trip(&sensors->soc_dts[dts_index], 1, 0, 0);
+ update_trip_temp(sensors, 0, 0);
+ update_trip_temp(sensors, 1, 0);
}
struct intel_soc_dts_sensors *
intel_soc_dts_iosf_init(enum intel_soc_dts_interrupt_type intr_type,
bool critical_trip, int crit_offset)
{
+ struct thermal_trip trips[SOC_MAX_DTS_SENSORS][SOC_MAX_DTS_TRIPS] = { 0 };
struct intel_soc_dts_sensors *sensors;
int tj_max;
int ret;
@@ -350,11 +336,13 @@ intel_soc_dts_iosf_init(enum intel_soc_d
sensors->soc_dts[i].sensors = sensors;
- ret = configure_trip(&sensors->soc_dts[i], 0,
- THERMAL_TRIP_PASSIVE, 0);
+ ret = update_trip_temp(sensors, 0, 0);
if (ret)
goto err_reset_trips;
+ trips[i][0].type = THERMAL_TRIP_PASSIVE;
+ trips[i][0].temperature = 0;
+
if (critical_trip) {
trip_type = THERMAL_TRIP_CRITICAL;
temp = sensors->tj_max - crit_offset;
@@ -362,13 +350,17 @@ intel_soc_dts_iosf_init(enum intel_soc_d
trip_type = THERMAL_TRIP_PASSIVE;
temp = 0;
}
- ret = configure_trip(&sensors->soc_dts[i], 1, trip_type, temp);
+ ret = update_trip_temp(sensors, 1, temp);
if (ret)
goto err_reset_trips;
+
+ trips[i][1].type = trip_type;
+ trips[i][1].temperature = temp;
}
for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
- ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], critical_trip);
+ ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], trips[i],
+ critical_trip);
if (ret)
goto err_remove_zone;
}
Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.h
+++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
@@ -29,7 +29,6 @@ struct intel_soc_dts_sensor_entry {
int id;
u32 store_status;
u32 trip_mask;
- struct thermal_trip trips[SOC_MAX_DTS_TRIPS];
struct thermal_zone_device *tzone;
struct intel_soc_dts_sensors *sensors;
};
Index: linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -53,7 +53,6 @@ struct zone_device {
u32 msr_pkg_therm_high;
struct delayed_work work;
struct thermal_zone_device *tzone;
- struct thermal_trip *trips;
struct cpumask cpumask;
};
@@ -268,17 +267,13 @@ static int pkg_thermal_notify(u64 msr_va
return 0;
}
-static struct thermal_trip *pkg_temp_thermal_trips_init(int cpu, int tj_max, int num_trips)
+static int pkg_temp_thermal_trips_init(int cpu, int tj_max,
+ struct thermal_trip *trips, int num_trips)
{
- struct thermal_trip *trips;
unsigned long thres_reg_value;
u32 mask, shift, eax, edx;
int ret, i;
- trips = kzalloc(sizeof(*trips) * num_trips, GFP_KERNEL);
- if (!trips)
- return ERR_PTR(-ENOMEM);
-
for (i = 0; i < num_trips; i++) {
if (i) {
@@ -291,10 +286,8 @@ static struct thermal_trip *pkg_temp_the
ret = rdmsr_on_cpu(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
&eax, &edx);
- if (ret < 0) {
- kfree(trips);
- return ERR_PTR(ret);
- }
+ if (ret < 0)
+ return ret;
thres_reg_value = (eax & mask) >> shift;
@@ -307,11 +300,12 @@ static struct thermal_trip *pkg_temp_the
__func__, cpu, i, trips[i].temperature);
}
- return trips;
+ return 0;
}
static int pkg_temp_thermal_device_add(unsigned int cpu)
{
+ struct thermal_trip trips[MAX_NUMBER_OF_TRIPS] = { 0 };
int id = topology_logical_die_id(cpu);
u32 eax, ebx, ecx, edx;
struct zone_device *zonedev;
@@ -336,21 +330,19 @@ static int pkg_temp_thermal_device_add(u
if (!zonedev)
return -ENOMEM;
- zonedev->trips = pkg_temp_thermal_trips_init(cpu, tj_max, thres_count);
- if (IS_ERR(zonedev->trips)) {
- err = PTR_ERR(zonedev->trips);
+ err = pkg_temp_thermal_trips_init(cpu, tj_max, trips, thres_count);
+ if (err)
goto out_kfree_zonedev;
- }
INIT_DELAYED_WORK(&zonedev->work, pkg_temp_thermal_threshold_work_fn);
zonedev->cpu = cpu;
zonedev->tzone = thermal_zone_device_register_with_trips("x86_pkg_temp",
- zonedev->trips, thres_count,
+ trips, thres_count,
(thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
if (IS_ERR(zonedev->tzone)) {
err = PTR_ERR(zonedev->tzone);
- goto out_kfree_trips;
+ goto out_kfree_zonedev;
}
err = thermal_zone_device_enable(zonedev->tzone);
if (err)
@@ -369,8 +361,6 @@ static int pkg_temp_thermal_device_add(u
out_unregister_tz:
thermal_zone_device_unregister(zonedev->tzone);
-out_kfree_trips:
- kfree(zonedev->trips);
out_kfree_zonedev:
kfree(zonedev);
return err;
@@ -457,10 +447,9 @@ static int pkg_thermal_cpu_offline(unsig
raw_spin_unlock_irq(&pkg_temp_lock);
/* Final cleanup if this is the last cpu */
- if (lastcpu) {
- kfree(zonedev->trips);
+ if (lastcpu)
kfree(zonedev);
- }
+
return 0;
}
From: Rafael J. Wysocki <[email protected]>
Because thermal zone operations are now stored directly in struct
thermal_zone_device, acpi_thermal_zone_ops need not be modified by
the thermal core and so it can be const.
Adjust the code accordingly.
No functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -620,7 +620,7 @@ acpi_thermal_unbind_cooling_device(struc
return acpi_thermal_bind_unbind_cdev(thermal, cdev, false);
}
-static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
+static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
.bind = acpi_thermal_bind_cooling_device,
.unbind = acpi_thermal_unbind_cooling_device,
.get_temp = thermal_get_temp,
On Mon, Feb 05, 2024 at 10:14:31PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The current code requires thermal zone creators to pass a pointer to a
> writable trips table to thermal_zone_device_register_with_trips() and
> that trips table is then used by the thermal core going forward.
>
> Consequently, the callers of thermal_zone_device_register_with_trips()
> are required to hold on to the trips table passed to it until the given
> thermal zone is unregistered, at which point the trips table can be
> freed, but at the same time they are not allowed to access the cells in
> that table directly. This is both error prone and confusing.
>
> To address it, turn the trips table pointer in struct thermal_zone_device
> into a flex array (counted by its num_trips field), allocate it during
> thermal zone device allocation and copy the contents of the trips table
> supplied by the zone creator (which can be const now) into it.
>
> This allows the callers of thermal_zone_device_register_with_trips() to
> drop their trip tables right after the zone registration.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Stanislaw Gruszka <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 16 +++++++++-------
> include/linux/thermal.h | 10 +++++-----
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -130,7 +130,6 @@ struct thermal_cooling_device {
> * @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
> * @mode: current mode of this thermal zone
> * @devdata: private pointer for device private data
> - * @trips: an array of struct thermal_trip
> * @num_trips: number of trip points the thermal zone supports
> * @passive_delay_jiffies: number of jiffies to wait between polls when
> * performing passive cooling.
> @@ -160,6 +159,7 @@ struct thermal_cooling_device {
> * @poll_queue: delayed work for polling
> * @notify_event: Last notification event
> * @suspended: thermal zone suspend indicator
> + * @trips: array of struct thermal_trip objects
> */
> struct thermal_zone_device {
> int id;
> @@ -172,7 +172,6 @@ struct thermal_zone_device {
> struct thermal_attr *trip_hyst_attrs;
> enum thermal_device_mode mode;
> void *devdata;
> - struct thermal_trip *trips;
> int num_trips;
> unsigned long passive_delay_jiffies;
> unsigned long polling_delay_jiffies;
> @@ -193,10 +192,11 @@ struct thermal_zone_device {
> struct list_head node;
> struct delayed_work poll_queue;
> enum thermal_notify_event notify_event;
> + bool suspended;
> #ifdef CONFIG_THERMAL_DEBUGFS
> struct thermal_debugfs *debugfs;
> #endif
> - bool suspended;
> + struct thermal_trip trips[] __counted_by(num_trips);
> };
>
> /**
> @@ -315,7 +315,7 @@ int thermal_zone_get_crit_temp(struct th
> #ifdef CONFIG_THERMAL
> struct thermal_zone_device *thermal_zone_device_register_with_trips(
> const char *type,
> - struct thermal_trip *trips,
> + const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> struct thermal_zone_device_ops *ops,
> @@ -375,7 +375,7 @@ void thermal_zone_device_critical(struct
> #else
> static inline struct thermal_zone_device *thermal_zone_device_register_with_trips(
> const char *type,
> - struct thermal_trip *trips,
> + const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> struct thermal_zone_device_ops *ops,
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1272,10 +1272,13 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
> * IS_ERR*() helpers.
> */
> struct thermal_zone_device *
> -thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int num_trips, int mask,
> - void *devdata, struct thermal_zone_device_ops *ops,
> - const struct thermal_zone_params *tzp, int passive_delay,
> - int polling_delay)
> +thermal_zone_device_register_with_trips(const char *type,
> + const struct thermal_trip *trips,
> + int num_trips, int mask,
> + void *devdata,
> + struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_params *tzp,
> + int passive_delay, int polling_delay)
> {
> struct thermal_zone_device *tz;
> int id;
> @@ -1322,7 +1325,7 @@ thermal_zone_device_register_with_trips(
> if (!thermal_class)
> return ERR_PTR(-ENODEV);
>
> - tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> + tz = kzalloc(struct_size(tz, trips, num_trips), GFP_KERNEL);
> if (!tz)
> return ERR_PTR(-ENOMEM);
>
> @@ -1344,7 +1347,6 @@ thermal_zone_device_register_with_trips(
> result = id;
> goto free_tzp;
> }
> -
> tz->id = id;
> strscpy(tz->type, type, sizeof(tz->type));
>
> @@ -1354,7 +1356,7 @@ thermal_zone_device_register_with_trips(
> tz->ops = ops;
> tz->device.class = thermal_class;
> tz->devdata = devdata;
> - tz->trips = trips;
> + memcpy(tz->trips, trips, num_trips * sizeof(trips[0]));
> tz->num_trips = num_trips;
>
> thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
>
>
>
>
On Mon, Feb 05, 2024 at 10:18:50PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Because thermal zone operations are now stored directly in struct
> thermal_zone_device, acpi_thermal_zone_ops need not be modified by
> the thermal core and so it can be const.
>
> Adjust the code accordingly.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Stanislaw Gruszka <[email protected]>
> ---
> drivers/acpi/thermal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -620,7 +620,7 @@ acpi_thermal_unbind_cooling_device(struc
> return acpi_thermal_bind_unbind_cdev(thermal, cdev, false);
> }
>
> -static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> +static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> .bind = acpi_thermal_bind_cooling_device,
> .unbind = acpi_thermal_unbind_cooling_device,
> .get_temp = thermal_get_temp,
>
>
>
>
On Mon, Feb 05, 2024 at 10:16:59PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Because the thermal core creates and uses its own copy of the trips
> table passed to thermal_zone_device_register_with_trips(), it is not
> necessary to hold on to a local copy of it any more after the given
> thermal zone has been registered.
>
> Accordingly, modify Intel thermal drivers to discard the trips tables
> passed to thermal_zone_device_register_with_trips() after thermal zone
> registration, for example by storing them in local variables which are
> automatically discarded when the zone registration is complete.
>
> Also make some additional code simplifications unlocked by the above
> changes.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Stanislaw Gruszka <[email protected]>
> ---
> drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 6 -
> drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h | 1
> drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c | 6 -
> drivers/thermal/intel/intel_pch_thermal.c | 21 ++---
> drivers/thermal/intel/intel_quark_dts_thermal.c | 12 +--
> drivers/thermal/intel/intel_soc_dts_iosf.c | 38 +++-------
> drivers/thermal/intel/intel_soc_dts_iosf.h | 1
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 35 +++------
> 8 files changed, 48 insertions(+), 72 deletions(-)
>
> Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -179,8 +179,6 @@ struct int34x_thermal_zone *int340x_ther
> for (i = 0; i < trip_cnt; ++i)
> zone_trips[i].hysteresis = hyst;
>
> - int34x_zone->trips = zone_trips;
> -
> int34x_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
>
> int34x_zone->zone = thermal_zone_device_register_with_trips(
> @@ -190,6 +188,8 @@ struct int34x_thermal_zone *int340x_ther
> int34x_zone->ops,
> &int340x_thermal_params,
> 0, 0);
> + kfree(zone_trips);
> +
> if (IS_ERR(int34x_zone->zone)) {
> ret = PTR_ERR(int34x_zone->zone);
> goto err_thermal_zone;
> @@ -203,7 +203,6 @@ struct int34x_thermal_zone *int340x_ther
> err_enable:
> thermal_zone_device_unregister(int34x_zone->zone);
> err_thermal_zone:
> - kfree(int34x_zone->trips);
> acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
> err_trips_alloc:
> kfree(int34x_zone->ops);
> @@ -217,7 +216,6 @@ void int340x_thermal_zone_remove(struct
> {
> thermal_zone_device_unregister(int34x_zone->zone);
> acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
> - kfree(int34x_zone->trips);
> kfree(int34x_zone->ops);
> kfree(int34x_zone);
> }
> Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> +++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> @@ -20,7 +20,6 @@ struct active_trip {
>
> struct int34x_thermal_zone {
> struct acpi_device *adev;
> - struct thermal_trip *trips;
> int aux_trip_nr;
> struct thermal_zone_device *zone;
> struct thermal_zone_device_ops *ops;
> Index: linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> +++ linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> @@ -233,10 +233,6 @@ static int get_trip_temp(struct proc_the
> return temp;
> }
>
> -static struct thermal_trip psv_trip = {
> - .type = THERMAL_TRIP_PASSIVE,
> -};
> -
> static struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> .set_trip_temp = sys_set_trip_temp,
> @@ -251,6 +247,7 @@ static int proc_thermal_pci_probe(struct
> {
> struct proc_thermal_device *proc_priv;
> struct proc_thermal_pci *pci_info;
> + struct thermal_trip psv_trip = { 0 };
> int irq_flag = 0, irq, ret;
> bool msi_irq = false;
>
> @@ -288,6 +285,7 @@ static int proc_thermal_pci_probe(struct
> }
>
> psv_trip.temperature = get_trip_temp(pci_info);
> + psv_trip.type = THERMAL_TRIP_PASSIVE;
>
> pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
> 1, 1, pci_info,
> Index: linux-pm/drivers/thermal/intel/intel_pch_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_pch_thermal.c
> +++ linux-pm/drivers/thermal/intel/intel_pch_thermal.c
> @@ -84,7 +84,6 @@ struct pch_thermal_device {
> void __iomem *hw_base;
> struct pci_dev *pdev;
> struct thermal_zone_device *tzd;
> - struct thermal_trip trips[PCH_MAX_TRIPS];
> bool bios_enabled;
> };
>
> @@ -94,7 +93,8 @@ struct pch_thermal_device {
> * passive trip temperature using _PSV method. There is no specific
> * passive temperature setting in MMIO interface of this PCI device.
> */
> -static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd, int trip)
> +static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd,
> + struct thermal_trip *trip)
> {
> struct acpi_device *adev;
> int temp;
> @@ -106,8 +106,8 @@ static int pch_wpt_add_acpi_psv_trip(str
> if (thermal_acpi_passive_trip_temp(adev, &temp) || temp <= 0)
> return 0;
>
> - ptd->trips[trip].type = THERMAL_TRIP_PASSIVE;
> - ptd->trips[trip].temperature = temp;
> + trip->type = THERMAL_TRIP_PASSIVE;
> + trip->temperature = temp;
> return 1;
> }
> #else
> @@ -159,6 +159,7 @@ static const char *board_names[] = {
> static int intel_pch_thermal_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> {
> + struct thermal_trip ptd_trips[PCH_MAX_TRIPS] = { 0 };
> enum pch_board_ids board_id = id->driver_data;
> struct pch_thermal_device *ptd;
> int nr_trips = 0;
> @@ -220,21 +221,21 @@ read_trips:
> trip_temp = readw(ptd->hw_base + WPT_CTT);
> trip_temp &= 0x1FF;
> if (trip_temp) {
> - ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> - ptd->trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
> + ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> + ptd_trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
> }
>
> trip_temp = readw(ptd->hw_base + WPT_PHL);
> trip_temp &= 0x1FF;
> if (trip_temp) {
> - ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> - ptd->trips[nr_trips++].type = THERMAL_TRIP_HOT;
> + ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> + ptd_trips[nr_trips++].type = THERMAL_TRIP_HOT;
> }
>
> - nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
> + nr_trips += pch_wpt_add_acpi_psv_trip(ptd, &ptd_trips[nr_trips]);
>
> ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
> - ptd->trips, nr_trips,
> + ptd_trips, nr_trips,
> 0, ptd, &tzd_ops,
> NULL, 0, 0);
> if (IS_ERR(ptd->tzd)) {
> Index: linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -105,7 +105,6 @@ struct soc_sensor_entry {
> u32 store_ptps;
> u32 store_dts_enable;
> struct thermal_zone_device *tzone;
> - struct thermal_trip trips[QRK_MAX_DTS_TRIPS];
> };
>
> static struct soc_sensor_entry *soc_dts;
> @@ -320,6 +319,7 @@ static void free_soc_dts(struct soc_sens
>
> static struct soc_sensor_entry *alloc_soc_dts(void)
> {
> + struct thermal_trip trips[QRK_MAX_DTS_TRIPS] = { 0 };
> struct soc_sensor_entry *aux_entry;
> int err;
> u32 out;
> @@ -362,14 +362,14 @@ static struct soc_sensor_entry *alloc_so
> goto err_ret;
> }
>
> - aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
> - aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
> + trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
> + trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
>
> - aux_entry->trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
> - aux_entry->trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
> + trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
> + trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
>
> aux_entry->tzone = thermal_zone_device_register_with_trips("quark_dts",
> - aux_entry->trips,
> + trips,
> QRK_MAX_DTS_TRIPS,
> wr_mask,
> aux_entry, &tzone_ops,
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> @@ -129,22 +129,6 @@ err_restore_ptps:
> return status;
> }
>
> -static int configure_trip(struct intel_soc_dts_sensor_entry *dts,
> - int thres_index, enum thermal_trip_type trip_type,
> - int temp)
> -{
> - int ret;
> -
> - ret = update_trip_temp(dts->sensors, thres_index, temp);
> - if (ret)
> - return ret;
> -
> - dts->trips[thres_index].temperature = temp;
> - dts->trips[thres_index].type = trip_type;
> -
> - return 0;
> -}
> -
> static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
> int temp)
> {
> @@ -218,6 +202,7 @@ static void remove_dts_thermal_zone(stru
> }
>
> static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
> + struct thermal_trip *trips,
> bool critical_trip)
> {
> int writable_trip_cnt = SOC_MAX_DTS_TRIPS;
> @@ -254,7 +239,7 @@ static int add_dts_thermal_zone(int id,
> }
> dts->trip_mask = trip_mask;
> snprintf(name, sizeof(name), "soc_dts%d", id);
> - dts->tzone = thermal_zone_device_register_with_trips(name, dts->trips,
> + dts->tzone = thermal_zone_device_register_with_trips(name, trips,
> SOC_MAX_DTS_TRIPS,
> trip_mask,
> dts, &tzone_ops,
> @@ -315,14 +300,15 @@ EXPORT_SYMBOL_GPL(intel_soc_dts_iosf_int
>
> static void dts_trips_reset(struct intel_soc_dts_sensors *sensors, int dts_index)
> {
> - configure_trip(&sensors->soc_dts[dts_index], 0, 0, 0);
> - configure_trip(&sensors->soc_dts[dts_index], 1, 0, 0);
> + update_trip_temp(sensors, 0, 0);
> + update_trip_temp(sensors, 1, 0);
> }
>
> struct intel_soc_dts_sensors *
> intel_soc_dts_iosf_init(enum intel_soc_dts_interrupt_type intr_type,
> bool critical_trip, int crit_offset)
> {
> + struct thermal_trip trips[SOC_MAX_DTS_SENSORS][SOC_MAX_DTS_TRIPS] = { 0 };
> struct intel_soc_dts_sensors *sensors;
> int tj_max;
> int ret;
> @@ -350,11 +336,13 @@ intel_soc_dts_iosf_init(enum intel_soc_d
>
> sensors->soc_dts[i].sensors = sensors;
>
> - ret = configure_trip(&sensors->soc_dts[i], 0,
> - THERMAL_TRIP_PASSIVE, 0);
> + ret = update_trip_temp(sensors, 0, 0);
> if (ret)
> goto err_reset_trips;
>
> + trips[i][0].type = THERMAL_TRIP_PASSIVE;
> + trips[i][0].temperature = 0;
> +
> if (critical_trip) {
> trip_type = THERMAL_TRIP_CRITICAL;
> temp = sensors->tj_max - crit_offset;
> @@ -362,13 +350,17 @@ intel_soc_dts_iosf_init(enum intel_soc_d
> trip_type = THERMAL_TRIP_PASSIVE;
> temp = 0;
> }
> - ret = configure_trip(&sensors->soc_dts[i], 1, trip_type, temp);
> + ret = update_trip_temp(sensors, 1, temp);
> if (ret)
> goto err_reset_trips;
> +
> + trips[i][1].type = trip_type;
> + trips[i][1].temperature = temp;
> }
>
> for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
> - ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], critical_trip);
> + ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], trips[i],
> + critical_trip);
> if (ret)
> goto err_remove_zone;
> }
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.h
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
> @@ -29,7 +29,6 @@ struct intel_soc_dts_sensor_entry {
> int id;
> u32 store_status;
> u32 trip_mask;
> - struct thermal_trip trips[SOC_MAX_DTS_TRIPS];
> struct thermal_zone_device *tzone;
> struct intel_soc_dts_sensors *sensors;
> };
> Index: linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -53,7 +53,6 @@ struct zone_device {
> u32 msr_pkg_therm_high;
> struct delayed_work work;
> struct thermal_zone_device *tzone;
> - struct thermal_trip *trips;
> struct cpumask cpumask;
> };
>
> @@ -268,17 +267,13 @@ static int pkg_thermal_notify(u64 msr_va
> return 0;
> }
>
> -static struct thermal_trip *pkg_temp_thermal_trips_init(int cpu, int tj_max, int num_trips)
> +static int pkg_temp_thermal_trips_init(int cpu, int tj_max,
> + struct thermal_trip *trips, int num_trips)
> {
> - struct thermal_trip *trips;
> unsigned long thres_reg_value;
> u32 mask, shift, eax, edx;
> int ret, i;
>
> - trips = kzalloc(sizeof(*trips) * num_trips, GFP_KERNEL);
> - if (!trips)
> - return ERR_PTR(-ENOMEM);
> -
> for (i = 0; i < num_trips; i++) {
>
> if (i) {
> @@ -291,10 +286,8 @@ static struct thermal_trip *pkg_temp_the
>
> ret = rdmsr_on_cpu(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
> &eax, &edx);
> - if (ret < 0) {
> - kfree(trips);
> - return ERR_PTR(ret);
> - }
> + if (ret < 0)
> + return ret;
>
> thres_reg_value = (eax & mask) >> shift;
>
> @@ -307,11 +300,12 @@ static struct thermal_trip *pkg_temp_the
> __func__, cpu, i, trips[i].temperature);
> }
>
> - return trips;
> + return 0;
> }
>
> static int pkg_temp_thermal_device_add(unsigned int cpu)
> {
> + struct thermal_trip trips[MAX_NUMBER_OF_TRIPS] = { 0 };
> int id = topology_logical_die_id(cpu);
> u32 eax, ebx, ecx, edx;
> struct zone_device *zonedev;
> @@ -336,21 +330,19 @@ static int pkg_temp_thermal_device_add(u
> if (!zonedev)
> return -ENOMEM;
>
> - zonedev->trips = pkg_temp_thermal_trips_init(cpu, tj_max, thres_count);
> - if (IS_ERR(zonedev->trips)) {
> - err = PTR_ERR(zonedev->trips);
> + err = pkg_temp_thermal_trips_init(cpu, tj_max, trips, thres_count);
> + if (err)
> goto out_kfree_zonedev;
> - }
>
> INIT_DELAYED_WORK(&zonedev->work, pkg_temp_thermal_threshold_work_fn);
> zonedev->cpu = cpu;
> zonedev->tzone = thermal_zone_device_register_with_trips("x86_pkg_temp",
> - zonedev->trips, thres_count,
> + trips, thres_count,
> (thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
> zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
> if (IS_ERR(zonedev->tzone)) {
> err = PTR_ERR(zonedev->tzone);
> - goto out_kfree_trips;
> + goto out_kfree_zonedev;
> }
> err = thermal_zone_device_enable(zonedev->tzone);
> if (err)
> @@ -369,8 +361,6 @@ static int pkg_temp_thermal_device_add(u
>
> out_unregister_tz:
> thermal_zone_device_unregister(zonedev->tzone);
> -out_kfree_trips:
> - kfree(zonedev->trips);
> out_kfree_zonedev:
> kfree(zonedev);
> return err;
> @@ -457,10 +447,9 @@ static int pkg_thermal_cpu_offline(unsig
> raw_spin_unlock_irq(&pkg_temp_lock);
>
> /* Final cleanup if this is the last cpu */
> - if (lastcpu) {
> - kfree(zonedev->trips);
> + if (lastcpu)
> kfree(zonedev);
> - }
> +
> return 0;
> }
>
>
>
>
>
On Thu, Feb 8, 2024 at 9:22 AM Stanislaw Gruszka
<[email protected]> wrote:
>
> On Mon, Feb 05, 2024 at 10:14:31PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > The current code requires thermal zone creators to pass a pointer to a
> > writable trips table to thermal_zone_device_register_with_trips() and
> > that trips table is then used by the thermal core going forward.
> >
> > Consequently, the callers of thermal_zone_device_register_with_trips()
> > are required to hold on to the trips table passed to it until the given
> > thermal zone is unregistered, at which point the trips table can be
> > freed, but at the same time they are not allowed to access the cells in
> > that table directly. This is both error prone and confusing.
> >
> > To address it, turn the trips table pointer in struct thermal_zone_device
> > into a flex array (counted by its num_trips field), allocate it during
> > thermal zone device allocation and copy the contents of the trips table
> > supplied by the zone creator (which can be const now) into it.
> >
> > This allows the callers of thermal_zone_device_register_with_trips() to
> > drop their trip tables right after the zone registration.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> Reviewed-by: Stanislaw Gruszka <[email protected]>
Thanks a lot for all of the reviews, much appreciated, especially
regarding the Intel drivers changes.
Unfortunately, this patch series, and the first half of it in
particular, is somewhat premature, because a couple of thermal drivers
do unexpected things to their trip point tables and they need to be
modified to stop accessing the trip tables directly before the core
can start using internal copies of them.
I'm going to save the tags for the future, however.