Hi Viresh, thanks for clarifying... On 22-May 15:53, Viresh Kumar wrote: > On 21-05-18, 10:20, Joel Fernandes wrote: > > On Mon, May 21, 2018 at 06:00:50PM +0100, Patrick Bellasi wrote: > > > If that's the case, this means that if, for example, during a > > > frequency switch you get a request to reduce the frequency (e.g. > > > deadline task passing the 0-lag time) and right after a request to > > > increase the frequency (e.g. the current FAIR task tick)... you will > > > enqueue a freq drop followed by a freq increase and actually do two > > > frequency hops? > > I don't think so. > > Consider the kthread as running currently and has just cleared the > work_in_progress flag. Sched update comes at that time and we decide > to reduce the frequency, we queue another work and update next_freq. > Now if another sched update comes before the kthread finishes its > previous loop, we will simply update next_freq and return. So when the > next time kthread runs, it will pick the most recent update.
Mmm... right... looking better at the two execution contexts: // A) Frequency update requests sugov_update_commit() { sg_policy->next_freq = next_freq; if (!sg_policy->work_in_progress) { sg_policy->work_in_progress = true; irq_work_queue(&sg_policy->irq_work); } } // B) Actual frequency updates sugov_work() { freq = sg_policy->next_freq; sg_policy->work_in_progress = false; __cpufreq_driver_target(sg_policy->policy, freq, CPUFREQ_RELATION_L); } It's true that A will enqueue only one B at the first next_freq update and then it will keep just updating the next_freq. Thus, we should be ensure to have always just one kwork pending in the queue. > Where is the problem both of you see ? Perhaps the confusion comes just from the naming of "work_in_progress", which is confusing since we use it now to represent that we enqueued a frequency change and we wait for the kwork to pick it up. Maybe it can help to rename it to something like kwork_queued or update_pending, update_queued... ? -- #include <best/regards.h> Patrick Bellasi