Re: [PATCH 3/3] thermal/intel_powerclamp: Convert to CPU hotplug state

[Petr Mladek]

On Thu 2016-10-27 13:27:36, Jacob Pan wrote:
> On Thu, 27 Oct 2016 17:17:26 +0200
> Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:
> 
> > On 2016-10-27 16:53:48 [+0200], Petr Mladek wrote:
> > > 
> > > In each case, I wonder if the problem is caused by the conversion
> > > to the kthread worker or by the CPU hotplug state conversion.  
> > 
> > drop the hotplug patch and you will see.
> > 
> Petr,
> 
> I dropped hp patch no long see the hang during suspend to s3. However,
> the problem seems to be this line,
> 
> diff --git a/drivers/thermal/intel_powerclamp.c
> b/drivers/thermal/intel_powerclamp.c index 390e50b..b61da57 100644
> --- a/drivers/thermal/intel_powerclamp.c
> +++ b/drivers/thermal/intel_powerclamp.c
> @@ -574,7 +574,7 @@ static void stop_power_clamp_worker(unsigned long
> cpu) */
>         del_timer_sync(&w_data->wakeup_timer);
>         clear_bit(w_data->cpu, cpu_clamping_mask);
> -       kthread_destroy_worker(w_data->worker);
> +//     kthread_destroy_worker(w_data->worker);
>  
>         w_data->worker = NULL;
>  }
> 
> If I do the above, everything works with S3 and CPU HP patch.
> 
> Inside kthread_destroy_worker()
> 	kthread_flush_worker(worker);
> never completes then blocks s3 entry!

The kthread_flush_worker() was not needed because the queue was empty
in this case (runtime checked). But it still hangs on

  kthread_destroy_worker()
    kthread_stop(task);


Then I tried to revert the conversion to the kthread worker
API (2nd patch from this patchset), see below. And it still
hangs during the suspend inside

	powerclamp_cpu_predown()
	  kthread_stop(*percpu_thread);


Note that both kthread_flush_worker() and kthread_stop()
waits until the kthread gets scheduled and do some job.
Also note that the kthread is bound to the given CPU.

My guess is that the kthread cannot be scheduled at this stage.
I wonder if the CPU is already partially down or that tasks
are freezed so that "normal" tasks are not scheduled at
this point. I am still trying to understand the code
related to suspend, cpu hotplug, and scheduler.


Just for record. Below is the conversion to the new
CPU hotplug state that can be applied on top
of the 1st patch from this patchset ("thermal/intel_powerclamp:
 Remove duplicated code that starts the kthread").
It allows to rule out the kthread worker API for the moment.