Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On Friday 13 Apr 2018 at 09:32:53 (+0530), Viresh Kumar wrote: [...] > And for the whole OPP discussion, perhaps we should have another > architecture specific callback which the scheduler can call to get a > ready-made energy model with all the structures filled in. That way > the OPP specific stuff will move to the architecture specific > callback. Yes, that's another possible solution indeed. Actually, it's already on the list of ideas to be dicussed in OSPM ;-) Thanks, Quentin
Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On Friday 13 Apr 2018 at 09:32:53 (+0530), Viresh Kumar wrote: [...] > And for the whole OPP discussion, perhaps we should have another > architecture specific callback which the scheduler can call to get a > ready-made energy model with all the structures filled in. That way > the OPP specific stuff will move to the architecture specific > callback. Yes, that's another possible solution indeed. Actually, it's already on the list of ideas to be dicussed in OSPM ;-) Thanks, Quentin
Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On 06-04-18, 16:36, Dietmar Eggemann wrote:
> diff --git a/include/linux/sched/energy.h b/include/linux/sched/energy.h
> +#if defined(CONFIG_SMP) && defined(CONFIG_PM_OPP)
> +extern struct sched_energy_model ** __percpu energy_model;
> +extern struct static_key_false sched_energy_present;
> +extern struct list_head sched_freq_domains;
> +
> +static inline bool sched_energy_enabled(void)
> +{
> + return static_branch_unlikely(_energy_present);
> +}
> +
> +static inline struct cpumask *freq_domain_span(struct freq_domain *fd)
> +{
> + return >span;
> +}
> +
> +extern void init_sched_energy(void);
> +
> +#define for_each_freq_domain(fdom) \
> + list_for_each_entry(fdom, _freq_domains, next)
> +
> +#else
> +struct freq_domain;
> +static inline bool sched_energy_enabled(void) { return false; }
> +static inline struct cpumask
> +*freq_domain_span(struct freq_domain *fd) { return NULL; }
> +static inline void init_sched_energy(void) { }
> +#define for_each_freq_domain(fdom) for (; fdom; fdom = NULL)
I am not sure if this is correct. fdom would normally be a local
uninitialized variable and with above we may end up running the loop
once with an invalid pointer. Maybe rewrite it as:
for (fdom = NULL; fdom; )
And for the whole OPP discussion, perhaps we should have another
architecture specific callback which the scheduler can call to get a
ready-made energy model with all the structures filled in. That way
the OPP specific stuff will move to the architecture specific
callback.
--
viresh
Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On 06-04-18, 16:36, Dietmar Eggemann wrote:
> diff --git a/include/linux/sched/energy.h b/include/linux/sched/energy.h
> +#if defined(CONFIG_SMP) && defined(CONFIG_PM_OPP)
> +extern struct sched_energy_model ** __percpu energy_model;
> +extern struct static_key_false sched_energy_present;
> +extern struct list_head sched_freq_domains;
> +
> +static inline bool sched_energy_enabled(void)
> +{
> + return static_branch_unlikely(_energy_present);
> +}
> +
> +static inline struct cpumask *freq_domain_span(struct freq_domain *fd)
> +{
> + return >span;
> +}
> +
> +extern void init_sched_energy(void);
> +
> +#define for_each_freq_domain(fdom) \
> + list_for_each_entry(fdom, _freq_domains, next)
> +
> +#else
> +struct freq_domain;
> +static inline bool sched_energy_enabled(void) { return false; }
> +static inline struct cpumask
> +*freq_domain_span(struct freq_domain *fd) { return NULL; }
> +static inline void init_sched_energy(void) { }
> +#define for_each_freq_domain(fdom) for (; fdom; fdom = NULL)
I am not sure if this is correct. fdom would normally be a local
uninitialized variable and with above we may end up running the loop
once with an invalid pointer. Maybe rewrite it as:
for (fdom = NULL; fdom; )
And for the whole OPP discussion, perhaps we should have another
architecture specific callback which the scheduler can call to get a
ready-made energy model with all the structures filled in. That way
the OPP specific stuff will move to the architecture specific
callback.
--
viresh
Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On 04/10/2018 01:54 PM, Peter Zijlstra wrote: On Fri, Apr 06, 2018 at 04:36:03PM +0100, Dietmar Eggemann wrote: + /* +* Build the energy model of one CPU, and link it to all CPUs +* in its frequency domain. This should be correct as long as +* they share the same micro-architecture. +*/ Aside from the whole PM_OPP question; you should assert that assumption. Put an explicit check for the uarch in and FAIL the init if that isn't met. I don't think it makes _ANY_ kind of sense to share a frequency domain across uarchs and we should be very clear we're not going to support anything like that. I know DynamiQ strictly speaking allows that, but since it's insane, we should consider that a bug in DynamiQ. Totally agree! We will add this assert. One open question of the current EAS design solved ;-)
Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On 04/10/2018 01:54 PM, Peter Zijlstra wrote: On Fri, Apr 06, 2018 at 04:36:03PM +0100, Dietmar Eggemann wrote: + /* +* Build the energy model of one CPU, and link it to all CPUs +* in its frequency domain. This should be correct as long as +* they share the same micro-architecture. +*/ Aside from the whole PM_OPP question; you should assert that assumption. Put an explicit check for the uarch in and FAIL the init if that isn't met. I don't think it makes _ANY_ kind of sense to share a frequency domain across uarchs and we should be very clear we're not going to support anything like that. I know DynamiQ strictly speaking allows that, but since it's insane, we should consider that a bug in DynamiQ. Totally agree! We will add this assert. One open question of the current EAS design solved ;-)
Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On Fri, Apr 06, 2018 at 04:36:03PM +0100, Dietmar Eggemann wrote: > + /* > + * Build the energy model of one CPU, and link it to all CPUs > + * in its frequency domain. This should be correct as long as > + * they share the same micro-architecture. > + */ Aside from the whole PM_OPP question; you should assert that assumption. Put an explicit check for the uarch in and FAIL the init if that isn't met. I don't think it makes _ANY_ kind of sense to share a frequency domain across uarchs and we should be very clear we're not going to support anything like that. I know DynamiQ strictly speaking allows that, but since it's insane, we should consider that a bug in DynamiQ.
Re: [RFC PATCH v2 2/6] sched: Introduce energy models of CPUs
On Fri, Apr 06, 2018 at 04:36:03PM +0100, Dietmar Eggemann wrote: > + /* > + * Build the energy model of one CPU, and link it to all CPUs > + * in its frequency domain. This should be correct as long as > + * they share the same micro-architecture. > + */ Aside from the whole PM_OPP question; you should assert that assumption. Put an explicit check for the uarch in and FAIL the init if that isn't met. I don't think it makes _ANY_ kind of sense to share a frequency domain across uarchs and we should be very clear we're not going to support anything like that. I know DynamiQ strictly speaking allows that, but since it's insane, we should consider that a bug in DynamiQ.
