| Message ID | 20190528103313.1343-50-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: add core scheduling support | expand |
>>> On 28.05.19 at 12:33, <jgross@suse.com> wrote: > --- a/xen/arch/x86/sysctl.c > +++ b/xen/arch/x86/sysctl.c > @@ -200,7 +200,8 @@ long arch_do_sysctl( > > case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE: > case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE: > - if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings < 2 ) > + if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings < 2 || > + sched_disable_smt_switching ) > { > ret = -EOPNOTSUPP; > break; I'm not convinced -EOPNOTSUPP is an appropriate error code for this new case. -EPERM, -EACCES, or -EIO would all seem more appropriate to me (and perhaps there are further ones). > --- a/xen/common/schedule.c > +++ b/xen/common/schedule.c > @@ -57,6 +57,7 @@ integer_param("sched_ratelimit_us", sched_ratelimit_us); > > /* Number of vcpus per struct sched_unit. */ > static unsigned int sched_granularity = 1; > +bool sched_disable_smt_switching; __read_mostly (perhaps also the pre-existing variable in context)? Jan
On 28/05/2019 13:44, Jan Beulich wrote: >>>> On 28.05.19 at 12:33, <jgross@suse.com> wrote: >> --- a/xen/arch/x86/sysctl.c >> +++ b/xen/arch/x86/sysctl.c >> @@ -200,7 +200,8 @@ long arch_do_sysctl( >> >> case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE: >> case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE: >> - if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings < 2 ) >> + if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings < 2 || >> + sched_disable_smt_switching ) >> { >> ret = -EOPNOTSUPP; >> break; > > I'm not convinced -EOPNOTSUPP is an appropriate error code for > this new case. -EPERM, -EACCES, or -EIO would all seem more > appropriate to me (and perhaps there are further ones). I think -EIO or -EBUSY would be the best fit. > >> --- a/xen/common/schedule.c >> +++ b/xen/common/schedule.c >> @@ -57,6 +57,7 @@ integer_param("sched_ratelimit_us", sched_ratelimit_us); >> >> /* Number of vcpus per struct sched_unit. */ >> static unsigned int sched_granularity = 1; >> +bool sched_disable_smt_switching; > > __read_mostly (perhaps also the pre-existing variable in context)? Yes, can do. Juergen
On Tue, 2019-05-28 at 13:52 +0200, Juergen Gross wrote: > On 28/05/2019 13:44, Jan Beulich wrote: > > > > > On 28.05.19 at 12:33, <jgross@suse.com> wrote: > > > --- a/xen/arch/x86/sysctl.c > > > +++ b/xen/arch/x86/sysctl.c > > > @@ -200,7 +200,8 @@ long arch_do_sysctl( > > > > > > case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE: > > > case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE: > > > - if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings > > > < 2 ) > > > + if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings > > > < 2 || > > > + sched_disable_smt_switching ) > > > { > > > ret = -EOPNOTSUPP; > > > break; > > > > I'm not convinced -EOPNOTSUPP is an appropriate error code for > > this new case. -EPERM, -EACCES, or -EIO would all seem more > > appropriate to me (and perhaps there are further ones). > > I think -EIO or -EBUSY would be the best fit. > I agree, with mild preference for EBUSY. Regards
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c index 3f06fecbd8..034d78fe67 100644 --- a/xen/arch/x86/sysctl.c +++ b/xen/arch/x86/sysctl.c @@ -200,7 +200,8 @@ long arch_do_sysctl( case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE: case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE: - if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings < 2 ) + if ( !cpu_has_htt || boot_cpu_data.x86_num_siblings < 2 || + sched_disable_smt_switching ) { ret = -EOPNOTSUPP; break; diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 3c85861b15..8607262a71 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -57,6 +57,7 @@ integer_param("sched_ratelimit_us", sched_ratelimit_us); /* Number of vcpus per struct sched_unit. */ static unsigned int sched_granularity = 1; +bool sched_disable_smt_switching; const cpumask_t *sched_res_mask = &cpumask_all; /* Various timer handlers. */ diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index b6496f57f6..7dc63c449b 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -1020,6 +1020,7 @@ static inline bool is_vcpu_online(const struct vcpu *v) } extern bool sched_smt_power_savings; +extern bool sched_disable_smt_switching; extern enum cpufreq_controller { FREQCTL_none, FREQCTL_dom0_kernel, FREQCTL_xen
When core or socket scheduling are active enabling or disabling smt is not possible as that would require a major host reconfiguration. Add a bool sched_disable_smt_switching which will be set for core or socket scheduling. Signed-off-by: Juergen Gross <jgross@suse.com> --- V1: new patch --- xen/arch/x86/sysctl.c | 3 ++- xen/common/schedule.c | 1 + xen/include/xen/sched.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-)