Re: [PATCH v2] sched: print information about scheduling granularity

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

On 20/04/2020 14:45, Jürgen Groß wrote:
> On 20.04.20 15:06, Sergey Dyasli wrote:
>> Currently it might be not obvious which scheduling mode (e.g. core-
>> scheduling) is being used by the scheduler. Alleviate this by printing
>> additional information about the selected granularity per-cpupool.
>>
>> Note: per-cpupool granularity selection is not implemented yet.
>>        The single global value is being used for each cpupool.
> 
> This is misleading. You are using the per-cpupool values, but they
> are all the same right now.

This is what I meant by my note, but I might need to improve the wording
since the current one looks ambiguous to you.

> 
>>
>> Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
>> ---
>> v2:
>> - print information on a separate line
>> - use per-cpupool granularity
>> - updated commit message
>>
>> CC: Juergen Gross <jgross@xxxxxxxx>
>> CC: Dario Faggioli <dfaggioli@xxxxxxxx>
>> CC: George Dunlap <george.dunlap@xxxxxxxxxx>
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>>   xen/common/sched/cpupool.c | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
>> index d40345b585..68106f6c15 100644
>> --- a/xen/common/sched/cpupool.c
>> +++ b/xen/common/sched/cpupool.c
>> @@ -40,6 +40,30 @@ 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 void sched_gran_print(enum sched_gran mode, unsigned int gran)
>> +{
>> +    char *str = "";
>> +
>> +    switch ( mode )
>> +    {
>> +    case SCHED_GRAN_cpu:
>> +        str = "cpu";
>> +        break;
>> +    case SCHED_GRAN_core:
>> +        str = "core";
>> +        break;
>> +    case SCHED_GRAN_socket:
>> +        str = "socket";
>> +        break;
>> +    default:
>> +        ASSERT_UNREACHABLE();
>> +        break;
>> +    }
> 
> With this addition it might make sense to have an array indexed by
> mode to get the string. This array could then be used in
> sched_select_granularity(), too.

I had thoughts about that, and with your suggestion looks like I need
to go and do it.

> 
>> +
>> +    printk("Scheduling granularity: %s, %u CPU%s per sched-resource\n",
>> +           str, gran, gran == 1 ? "" : "s");
>> +}
>> +
>>   #ifdef CONFIG_HAS_SCHED_GRANULARITY
>>   static int __init sched_select_granularity(const char *str)
>>   {
>> @@ -115,6 +139,7 @@ static void __init cpupool_gran_init(void)
>>           warning_add(fallback);
>>       sched_granularity = gran;
>> +    sched_gran_print(opt_sched_granularity, sched_granularity);
>>   }
>>   unsigned int cpupool_get_granularity(const struct cpupool *c)
>> @@ -911,6 +936,7 @@ void dump_runq(unsigned char key)
>>       {
>>           printk("Cpupool %d:\n", (*c)->cpupool_id);
>>           printk("Cpus: %*pbl\n", CPUMASK_PR((*c)->cpu_valid));
>> +        sched_gran_print((*c)->gran, cpupool_get_granularity(*c));
>>           schedule_dump(*c);
>>       }
> 

--
Thanks,
Sergey