Re: [PATCH] sched: print information about scheduler granularity

[Sergey Dyasli <sergey.dyasli@citrix.com>]

On 17/04/2020 08:57, Jürgen Groß wrote:
> On 16.04.20 18:43, Dario Faggioli wrote:
>> On Thu, 2020-04-16 at 09:33 +0100, Sergey Dyasli wrote:
>>> Currently it might be not obvious which scheduling mode is being used
>>> by the scheduler. Alleviate this by printing additional information
>>> about the selected granularity.
>>>
>> I like the idea. However, I don't like how verbose and long that line
>> becomes.
>>
>>>   Messages now look like these:
>>>
>>> 1. boot
>>> (XEN) [00089808f0ea7496] Using scheduler: SMP Credit Scheduler
>>> (credit) in core-scheduling mode
>>>
>>> 2. xl debug-keys r
>>> (XEN) [   45.914314] Scheduler: SMP Credit Scheduler (credit) in 2-
>>> way core-scheduling mode
>>>
>> What about adding an entry, just below these ones. Something looking
>> like, for instance (both at boot and in the debug-key dump):
>>
>> "Scheduling granularity: cpu"
>>
>> (or "core", or "socket")

I agree that the line becomes too long. I'll print the new information
on a separate line as you suggest in v2.

>>
>> Also
>>
>>> --- a/xen/common/sched/cpupool.c
>>> +++ b/xen/common/sched/cpupool.c
>>> @@ -38,7 +38,35 @@ static cpumask_t cpupool_locked_cpus;
>>>   static DEFINE_SPINLOCK(cpupool_lock);
>>>   static enum sched_gran __read_mostly opt_sched_granularity =
>>> SCHED_GRAN_cpu;
>>> -static unsigned int __read_mostly sched_granularity = 1;
>>> +static unsigned int __read_mostly sched_granularity;
>>> +
>>> +char *sched_gran_str(char *str, size_t size)
>>> +{
>>> +    char *mode = "";
>>> +
>>> +    switch ( opt_sched_granularity )
>>> +    {
>>> +    case SCHED_GRAN_cpu:
>>> +        mode = "cpu";
>>> +        break;
>>> +    case SCHED_GRAN_core:
>>> +        mode = "core";
>>> +        break;
>>> +    case SCHED_GRAN_socket:
>>> +        mode = "socket";
>>> +        break;
>>> +    default:
>>> +        ASSERT_UNREACHABLE();
>>> +        break;
>>> +    }
>>> +
>>> +    if ( sched_granularity )
>>> +        snprintf(str, size, "%u-way %s", sched_granularity, mode);
>>>
>> I'm not sure about using the value of the enum like this.
> 
> enum? sched_granularity holds the number of cpus per scheduling
> resource. opt_sched_granularity is the enum.
> 
>>
>> E.g., in a system with 4 threads per core, enabling core scheduling
>> granularity would mean having 4 vCPUs in the scheduling units. But this
>> will still print "2-way core-scheduling", which I think would sound
>> confusing.
> 
> It would print "4-way", of course.
> 
>>
>> So I'd just go with "cpu", "core" and "socket" strings.
> 
> No, this is not a good idea. With e.g. smt=0 you'll be able to have
> "1-way core" which is much more informative than "core".

Can confirm the above. "sched-gran=core" on a Knights Mill produces:
(XEN) [  232.018648] Scheduler: SMP Credit Scheduler (credit) in 4-way core-scheduling mode

While "sched-gran=core smt=0" gives:
(XEN) [  259.337588] Scheduler: SMP Credit Scheduler (credit) in 1-way core-scheduling mode

--
Thanks,
Sergey