Re: [PATCH v6 1/2] sched/uclamp: Add a new sysctl to control RT default boost value

From: Valentin Schneider
Date: Wed Jul 08 2020 - 17:45:23 EST



On 08/07/20 14:08, Qais Yousef wrote:
> On 07/08/20 12:05, Valentin Schneider wrote:
>> > AFAIU rcu_read_lock() is light weight. So having the protection applied is more
>> > robust against future changes.
>>
>> So I think the one thing you win by having this dance with mb's and the
>> suggested handling of the task list is that you do not need any
>> rcu_synchronize() anymore. Both approaches have merit, it's just that the
>> way I understood the suggestion to add sched_post_fork() was to simplify
>> the ordering of the update with the aforementioned scheme.
>
> The synchronize_rcu() is not for sched_post_fork(). It is to deal with the
> preemption problem.
>
>>
>> >
>> >>
>> >> sched_post_fork() being preempted out is a bit more annoying, but what
>> >> prevents us from making that bit preempt-disabled?
>> >
>> > preempt_disable() is not friendly to RT and heavy handed approach IMO.
>> >
>>
>> True, but this is both an infrequent and slow sysctl path, so I don't think
>> RT would care much.
>
> There's an easy answer for that. But first I'm not sure what problem are we
> discussing here.
>
> What is the problem with rcu? And how is preempt_disable() fixes it or improves
> on it?
>

Minimizing the races we have to think and take care of would make this
easier to review and maintain. That's what I was suggesting with getting
entirely rid of the preemption+update issue and having only the
update+forkee race to take care of, which is IMO fairly straightforward to
reason about on its own.

That said, you're driving the thing, and I'm not, so I'll trust your
judgement here.

> Thanks