On Thu, 2016-05-12 at 23:19 -0700, Eduardo Valentin wrote:
> The current throttling strategy is to apply cooling levels
> on all processors in a package. Therefore, if one cooling
> device is requested to cap the frequency, the same request
> is applied to all sibling processors in the same package.
>
> For this reason, this patch removes the redundant cooling
> devices, registering only one cooling device per package.
>
> Signed-off-by: Eduardo Valentin <edubez...@gmail.com>
> ---
> Srinivas,
>
> This is the outcome of our discussion on the Processor cooling
> device.
> As per your suggestion, I am now attempting to register a single
> cooling device per package.
>
> The only thing is about the sysfs links. Now the Processor cooling
> device would look like:
> $ ls /sys/class/thermal/cooling_device0/
> cur_state device.0 device.1 device.2 device.3 max_state power
> subsystem type uevent
>
> Instead of:
>
> $ ls /sys/class/thermal/cooling_device0/
> cur_state device max_state power subsystem type uevent
>
> This is obviously to keep a link between the cooling device and
> its affected devices. Would this break userspace?
>
Conceptually it is fine and should work. I need to run some tests
before I confirm.
Hopefully I can do in next 2 weeks.
Thanks,
Srinivas
> BR,
>
> Eduardo Valentin
>
> drivers/acpi/processor_driver.c | 40
> +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c
> b/drivers/acpi/processor_driver.c
> index 11154a3..e1e17de 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -160,9 +160,29 @@ static struct notifier_block acpi_cpu_notifier =
> {
> };
>
> #ifdef CONFIG_ACPI_CPU_FREQ_PSS
> +static struct thermal_cooling_device *
> +acpi_pss_register_cooling(struct acpi_processor *pr, struct
> acpi_device *device)
> +{
> + int i, cpu = pr->id;
> +
> + for_each_online_cpu(i) {
> + if (topology_physical_package_id(i) ==
> + topology_physical_package_id(cpu)) {
> + struct acpi_processor *pre =
> per_cpu(processors, i);
> +
> + if (pre->cdev && !IS_ERR(pre->cdev))
> + return pre->cdev;
> + }
> + }
> +
> + return thermal_cooling_device_register("Processor", device,
> + &processor_cooling_op
> s);
> +}
> +
> static int acpi_pss_perf_init(struct acpi_processor *pr,
> struct acpi_device *device)
> {
> + char cpu_id[15];
> int result = 0;
>
> acpi_processor_ppc_has_changed(pr, 0);
> @@ -172,8 +192,7 @@ static int acpi_pss_perf_init(struct
> acpi_processor *pr,
> if (pr->flags.throttling)
> pr->flags.limit = 1;
>
> - pr->cdev = thermal_cooling_device_register("Processor",
> device,
> - &processor_coolin
> g_ops);
> + pr->cdev = acpi_pss_register_cooling(pr, device);
> if (IS_ERR(pr->cdev)) {
> result = PTR_ERR(pr->cdev);
> return result;
> @@ -191,9 +210,10 @@ static int acpi_pss_perf_init(struct
> acpi_processor *pr,
> goto err_thermal_unregister;
> }
>
> + snprintf(cpu_id, sizeof(cpu_id), "device.%d", pr->id);
> result = sysfs_create_link(&pr->cdev->device.kobj,
> &device->dev.kobj,
> - "device");
> + cpu_id);
> if (result) {
> dev_err(&pr->cdev->device,
> "Failed to create sysfs link 'device'\n");
> @@ -213,11 +233,25 @@ static int acpi_pss_perf_init(struct
> acpi_processor *pr,
> static void acpi_pss_perf_exit(struct acpi_processor *pr,
> struct acpi_device *device)
> {
> + char cpu_id[15];
> + int i;
> +
> if (pr->cdev) {
> sysfs_remove_link(&device->dev.kobj,
> "thermal_cooling");
> + snprintf(cpu_id, sizeof(cpu_id), "device.%d", pr-
> >id);
> sysfs_remove_link(&pr->cdev->device.kobj, "device");
> thermal_cooling_device_unregister(pr->cdev);
> pr->cdev = NULL;
> +
> + for_each_online_cpu(i) {
> + if (topology_physical_package_id(i) ==
> + topology_physical_package_id(pr->id)) {
> + struct acpi_processor *pre;
> +
> + pre = per_cpu(processors, i);
> + pre->cdev = NULL;
> + }
> + }
> }
> }
> #else
