[PATCH 2/3] thermal/intel_powerclamp: Convert the kthread to kthread worker API

[Petr Mladek]

Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.

The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.

The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.

This patch converts the intel powerclamp kthreads into the kthread
worker because they need to have a good control over the assigned
CPUs.

IMHO, the most natural way is to split one cycle into two works.
First one does some balancing and let the CPU work normal
way for some time. The second work checks what the CPU has done
in the meantime and put it into C-state to reach the required
idle time ratio. The delay between the two works is achieved
by the delayed kthread work.

The two works have to share some data that used to be local
variables of the single kthread function. This is achieved
by the new per-CPU struct kthread_worker_data. It might look
as a complication. On the other hand, the long original kthread
function was not nice either.

The patch tries to avoid extra init and cleanup works. All the
actions might be done outside the thread. They are moved
to the functions that create or destroy the worker. Especially,
I checked that the timers are assigned to the right CPU.

The two works are queuing each other. It makes it a bit tricky to
break it when we want to stop the worker. We use the global and
per-worker "clamping" variables to make sure that the re-queuing
eventually stops. We also cancel the works to make it faster.
Note that the canceling is not reliable because the handling
of the two variables and queuing is not synchronized via a lock.
But it is not a big deal because it is just an optimization.
The job is stopped faster than before in most cases.

Signed-off-by: Petr Mladek 
CC: Zhang Rui 
CC: Eduardo Valentin 
CC: Jacob Pan 
CC: Sebastian Andrzej Siewior 
CC: linux...@vger.kernel.org
Acked-by: Jacob Pan 
---
 drivers/thermal/intel_powerclamp.c | 292 +
 1 file changed, 170 insertions(+), 122 deletions(-)

diff --git a/drivers/thermal/intel_powerclamp.c 
b/drivers/thermal/intel_powerclamp.c
index 63657d193db5..a94f7c849a4e 100644
--- a/drivers/thermal/intel_powerclamp.c
+++ b/drivers/thermal/intel_powerclamp.c
@@ -86,11 +86,27 @@
  */
 static bool clamping;
 
+static const struct sched_param sparam = {
+   .sched_priority = MAX_USER_RT_PRIO / 2,
+};
+struct powerclamp_worker_data {
+   struct kthread_worker *worker;
+   struct kthread_work balancing_work;
+   struct kthread_delayed_work idle_injection_work;
+   struct timer_list wakeup_timer;
+   unsigned int cpu;
+   unsigned int count;
+   unsigned int guard;
+   unsigned int window_size_now;
+   unsigned int target_ratio;
+   unsigned int duration_jiffies;
+   bool clamping;
+};
 
-static struct task_struct * __percpu *powerclamp_thread;
+static struct powerclamp_worker_data * __percpu worker_data;
 static struct thermal_cooling_device *cooling_dev;
 static unsigned long *cpu_clamping_mask;  /* bit map for tracking per cpu
-  * clamping thread
+  * clamping kthread worker
   */
 
 static unsigned int duration;
@@ -368,103 +384,104 @@ static bool powerclamp_adjust_controls(unsigned int 
target_ratio,
return set_target_ratio + guard <= current_ratio;
 }
 
-static int clamp_thread(void *arg)
+static void clamp_balancing_func(struct kthread_work *work)
 {
-   int cpunr = (unsigned long)arg;
-   DEFINE_TIMER(wakeup_timer, noop_timer, 0, 0);
-   static const struct sched_param param = {
-   .sched_priority = MAX_USER_RT_PRIO/2,
-   };
-   unsigned int count = 0;
-   unsigned int target_ratio;
+   struct powerclamp_worker_data *w_data;
+   int sleeptime;
+   unsigned long target_jiffies;
+   unsigned int compensated_ratio;
+   int interval; /* jiffies to sleep for each attempt */
 
-   set_bit(cpunr, cpu_clamping_mask);
-   set_freezable();
-   init_timer_on_stack(_timer);
-   sched_setscheduler(current, SCHED_FIFO, );
-
-   while (true == clamping && !kthread_should_stop() &&
-   cpu_online(cpunr)) {
-   int sleeptime;
-   

[PATCH 2/3] thermal/intel_powerclamp: Convert the kthread to kthread worker API

[Petr Mladek]

Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.

The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.

The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.

This patch converts the intel powerclamp kthreads into the kthread
worker because they need to have a good control over the assigned
CPUs.

IMHO, the most natural way is to split one cycle into two works.
First one does some balancing and let the CPU work normal
way for some time. The second work checks what the CPU has done
in the meantime and put it into C-state to reach the required
idle time ratio. The delay between the two works is achieved
by the delayed kthread work.

The two works have to share some data that used to be local
variables of the single kthread function. This is achieved
by the new per-CPU struct kthread_worker_data. It might look
as a complication. On the other hand, the long original kthread
function was not nice either.

The patch tries to avoid extra init and cleanup works. All the
actions might be done outside the thread. They are moved
to the functions that create or destroy the worker. Especially,
I checked that the timers are assigned to the right CPU.

The two works are queuing each other. It makes it a bit tricky to
break it when we want to stop the worker. We use the global and
per-worker "clamping" variables to make sure that the re-queuing
eventually stops. We also cancel the works to make it faster.
Note that the canceling is not reliable because the handling
of the two variables and queuing is not synchronized via a lock.
But it is not a big deal because it is just an optimization.
The job is stopped faster than before in most cases.

Signed-off-by: Petr Mladek 
CC: Zhang Rui 
CC: Eduardo Valentin 
CC: Jacob Pan 
CC: Sebastian Andrzej Siewior 
CC: linux...@vger.kernel.org
Acked-by: Jacob Pan 
---
 drivers/thermal/intel_powerclamp.c | 292 +
 1 file changed, 170 insertions(+), 122 deletions(-)

diff --git a/drivers/thermal/intel_powerclamp.c 
b/drivers/thermal/intel_powerclamp.c
index 63657d193db5..a94f7c849a4e 100644
--- a/drivers/thermal/intel_powerclamp.c
+++ b/drivers/thermal/intel_powerclamp.c
@@ -86,11 +86,27 @@
  */
 static bool clamping;
 
+static const struct sched_param sparam = {
+   .sched_priority = MAX_USER_RT_PRIO / 2,
+};
+struct powerclamp_worker_data {
+   struct kthread_worker *worker;
+   struct kthread_work balancing_work;
+   struct kthread_delayed_work idle_injection_work;
+   struct timer_list wakeup_timer;
+   unsigned int cpu;
+   unsigned int count;
+   unsigned int guard;
+   unsigned int window_size_now;
+   unsigned int target_ratio;
+   unsigned int duration_jiffies;
+   bool clamping;
+};
 
-static struct task_struct * __percpu *powerclamp_thread;
+static struct powerclamp_worker_data * __percpu worker_data;
 static struct thermal_cooling_device *cooling_dev;
 static unsigned long *cpu_clamping_mask;  /* bit map for tracking per cpu
-  * clamping thread
+  * clamping kthread worker
   */
 
 static unsigned int duration;
@@ -368,103 +384,104 @@ static bool powerclamp_adjust_controls(unsigned int 
target_ratio,
return set_target_ratio + guard <= current_ratio;
 }
 
-static int clamp_thread(void *arg)
+static void clamp_balancing_func(struct kthread_work *work)
 {
-   int cpunr = (unsigned long)arg;
-   DEFINE_TIMER(wakeup_timer, noop_timer, 0, 0);
-   static const struct sched_param param = {
-   .sched_priority = MAX_USER_RT_PRIO/2,
-   };
-   unsigned int count = 0;
-   unsigned int target_ratio;
+   struct powerclamp_worker_data *w_data;
+   int sleeptime;
+   unsigned long target_jiffies;
+   unsigned int compensated_ratio;
+   int interval; /* jiffies to sleep for each attempt */
 
-   set_bit(cpunr, cpu_clamping_mask);
-   set_freezable();
-   init_timer_on_stack(_timer);
-   sched_setscheduler(current, SCHED_FIFO, );
-
-   while (true == clamping && !kthread_should_stop() &&
-   cpu_online(cpunr)) {
-   int sleeptime;
-   unsigned long target_jiffies;
-   unsigned int guard;
-   unsigned int compensated_ratio;
-   int interval;