On Wed, Jan 23, 2019 at 02:14:26PM +0000, Patrick Bellasi wrote: > > > Consider also that the uclamp_task_update_active() added by this patch > > > not only has lower overhead but it will be use also by cgroups where > > > we want to force update all the tasks on a cgroup's clamp change. > > > > I haven't gotten that far; but I would prefer not to have two different > > 'change' paths in __sched_setscheduler(). > > Yes, I agree that two paths in __sched_setscheduler() could be > confusing. Still we have to consider that here we are adding > "not class specific" attributes. But that change thing is not class specific; the whole: rq = task_rq_lock(p, &rf); queued = task_on_rq_queued(p); running = task_current(rq, p); if (queued) dequeue_task(rq, p, queue_flags); if (running) put_prev_task(rq, p); /* @p is in it's invariant state; frob it's state */ if (queued) enqueue_task(rq, p, queue_flags); if (running) set_curr_task(rq, p); task_rq_unlock(rq, p, &rf); pattern is all over the place; it is just because C sucks that that isn't more explicitly shared (do_set_cpus_allowed(), rt_mutex_setprio(), set_user_nice(), __sched_setscheduler(), sched_setnuma(), sched_move_task()). This is _the_ pattern for changing state and is not class specific at all.