Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1528949

Re: [PATCH V2 0/4] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

Show all headers | View raw

On 23-11-16, 19:01, Steve Muckle wrote:
> I know this has already gone in, but can you expand on the unmet
> guarantees mentioned here just for my own (and perhaps others')
> understanding?

Sure. This is the simplified form of your original patch:

@@ -71,7 +73,7 @@ static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time)
 {
        s64 delta_ns;
 
-       if (sg_policy->work_in_progress)
+       if (sg_policy->thread_active)
                return false;
 
        if (unlikely(sg_policy->need_freq_update)) {
 
+static int sugov_thread(void *data)
 {
	...

+       do {

        ...

+               set_current_state(TASK_RUNNING);
+               /* Issue request. */
+               mutex_lock(&sg_policy->slow_lock);
+               __cpufreq_driver_target(sg_policy->policy,
+                                       sg_policy->next_freq,
+                                       CPUFREQ_RELATION_L);
+               mutex_unlock(&sg_policy->slow_lock);
+
+               sg_policy->thread_active = false;

        ...

+               schedule();
+
+       } while (!kthread_should_stop());
+       return 0;
 }
 
 static void sugov_irq_work(struct irq_work *irq_work)
@@ -349,7 +382,7 @@ static void sugov_irq_work(struct irq_work *irq_work)
        struct sugov_policy *sg_policy;
 
        sg_policy = container_of(irq_work, struct sugov_policy, irq_work);
-       schedule_work_on(smp_processor_id(), &sg_policy->work);
+       wake_up_process(sg_policy->thread);
 }

Consider that the thread has just got a chance to run and has set
'thread_active' to false from sugov_thread(), i.e. schedule() isn't called yet.

At this time if sugov_should_update_freq() gets called again, it will return
'true', and eventually we will land into sugov_irq_work() and that will call
wake_up_process(). But the process is already running and haven't slept yet.
I am not sure how it works but I don't expect the thread to go to sleep again at
this point of time.

And in this particular case we will end up not evaluating the load and doing
DVFS for period = 2 * rate_limit_us, whereas we wanted to do that every
rate_limit microseconds.

Of course a simple kthread would have been better instead of a kthread + work if
this wasn't the case.

Does that sound reasonable or Am I missing something ?

-- 
viresh

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH V2 0/4] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task Viresh Kumar <viresh.kumar@linaro.org> - 2016-11-15 09:30 +0100
  [PATCH V2 2/4] cpufreq: schedutil: enable fast switch earlier Viresh Kumar <viresh.kumar@linaro.org> - 2016-11-15 09:30 +0100
  [PATCH V2 4/4] cpufreq: schedutil: irq-work and mutex are only used in slow path Viresh Kumar <viresh.kumar@linaro.org> - 2016-11-15 09:30 +0100
    Re: [PATCH V2 4/4] cpufreq: schedutil: irq-work and mutex are only  used in slow path Ingo Molnar <mingo@kernel.org> - 2016-11-24 06:00 +0100
      Re: [PATCH V2 4/4] cpufreq: schedutil: irq-work and mutex are only  used in slow path Viresh Kumar <viresh.kumar@linaro.org> - 2016-11-24 07:20 +0100
        Re: [PATCH V2 4/4] cpufreq: schedutil: irq-work and mutex are only  used in slow path Ingo Molnar <mingo@kernel.org> - 2016-11-24 07:30 +0100
  [PATCH V2 3/4] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task Viresh Kumar <viresh.kumar@linaro.org> - 2016-11-15 09:30 +0100
    Re: [PATCH V2 3/4] cpufreq: schedutil: move slow path from workqueue  to SCHED_FIFO task Peter Zijlstra <peterz@infradead.org> - 2016-11-16 16:30 +0100
      Re: [PATCH V2 3/4] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-11-24 02:20 +0100
    Re: [PATCH V2 3/4] cpufreq: schedutil: move slow path from workqueue  to SCHED_FIFO task Ingo Molnar <mingo@kernel.org> - 2016-11-24 06:00 +0100
      Re: [PATCH V2 3/4] cpufreq: schedutil: move slow path from workqueue  to SCHED_FIFO task Viresh Kumar <viresh.kumar@linaro.org> - 2016-11-24 06:20 +0100
  Re: [PATCH V2 0/4] cpufreq: schedutil: move slow path from workqueue  to SCHED_FIFO task Steve Muckle <smuckle.linux@gmail.com> - 2016-11-24 04:10 +0100
    Re: [PATCH V2 0/4] cpufreq: schedutil: move slow path from workqueue  to SCHED_FIFO task Viresh Kumar <viresh.kumar@linaro.org> - 2016-11-24 05:10 +0100

csiph-web