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

[Jürgen Groß <jgross@xxxxxxxx>]

On 17.04.20 15:43, Sergey Dyasli wrote:
> 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
You might want to consider not using the global variables
[opt_]sched_granularity, but the per-cpupool ones. Right now they have
the same value, but this might change in future...


Juergen