Re: config SCHED_CORE

[Randy Dunlap]

On 5/23/21 1:30 PM, Hugh Dickins wrote:
> On Sun, 23 May 2021, Peter Zijlstra wrote:
>> On Fri, May 21, 2021 at 07:57:35AM -0400, Joel Fernandes wrote:
>>> On Fri, May 21, 2021 at 3:53 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>>>> +       help
>>>> +         This option enables Core scheduling, a means of coordinated task
>>>> +         selection across SMT siblings with the express purpose of creating a
>>>> +         Core wide privilidge boundary. When enabled -- see prctl(PR_SCHED_CORE)
>>>> +         -- task selection will ensure all SMT siblings will execute a task
>>>> +         from the same 'core group', forcing idle when no matching task is found.
>>>> +
>>>> +         This provides means of mitigation against a number of SMT side-channels;
>>>> +         but is, on its own, insufficient to mitigate all known side-channels.
>>>> +         Notable: the MDS class of attacks require more.
>>>> +
>>>> +         Default enabled for anything that has SCHED_SMT, when unused there should
>>>> +         be no impact on performance.
>>>
>>> This description sort of makes it sound like security is the only
>>> usecase. Perhaps we can also add here that core-scheduling can help
>>> performance of workloads where hyperthreading is undesired, such as
>>> when VM providers don't want to share hyperthreads.
>>
>> Something like so then?
> 
> Much more helpful, thanks. And I agree that you have to keep it fairly
> brief here: I think you've struck the right balance.  Some nits below.
> 
>>
>> ---
>>  kernel/Kconfig.preempt | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
>> index ea1e3331c0ba..cd497fecfd43 100644
>> --- a/kernel/Kconfig.preempt
>> +++ b/kernel/Kconfig.preempt
>> @@ -104,4 +104,18 @@ config SCHED_CORE
>>  	bool "Core Scheduling for SMT"
>>  	default y
>>  	depends on SCHED_SMT
>> +	help
>> +	  This option enables Core scheduling, a means of coordinated task
> 
> Maybe s/scheduling/Scheduling/ to match the title?
> 
> I think I got the picture once I reached the end, but was confused here
> by the stages of enablement.  s/This option enables/This option permits/
> would be clearer, I think.
> 

I like all of Hugh's suggestions...

> 
>> +	  selection across SMT siblings. When enabled -- see
>> +	  prctl(PR_SCHED_CORE) -- task selection will ensure all SMT siblings
> 
> s/will ensure/ensures that/ (it felt like too many "will"s before)

especially that one. ^^^

>> +	  will execute a task from the same 'core group', forcing idle when no
>> +	  matching task is found.
>> +
>> +	  Use of this feature includes:
>> +	   - mitigation of some (not all) SMT side channels;
>> +	   - limiting SMT interference to improve determinism and/or performance.
>> +
>> +	  Default enabled for anything that has SCHED_SMT, when unused there
> 
> "SCHED_CORE is default enabled when SCHED_SMT is enabled - when unused there"
> would be better.
> 
>> +	  should be no impact on performance.


thanks.
-- 
~Randy