| Message ID | 1413487800-7162-4-git-send-email-kernelfans@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Hi Liu, On 10/17/2014 12:59 AM, kernelfans@gmail.com wrote: > When kvm is enabled on a core, we migrate all external irq to primary > thread. Since currently, the kvmirq logic is handled by the primary > hwthread. > > Todo: this patch lacks re-enable of irqbalance when kvm is disable on > the core Why is a sysfs file introduced to trigger irq migration? Why is it not done during kvm module insert ? And similarly spread interrupts when the module is removed? Isn't this a saner way ? > > Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/sysfs.c | 39 ++++++++++++++++++++++++++++++++++ > arch/powerpc/sysdev/xics/xics-common.c | 12 +++++++++++ > 2 files changed, 51 insertions(+) > > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c > index 67fd2fd..a2595dd 100644 > --- a/arch/powerpc/kernel/sysfs.c > +++ b/arch/powerpc/kernel/sysfs.c > @@ -552,6 +552,45 @@ static void sysfs_create_dscr_default(void) > if (cpu_has_feature(CPU_FTR_DSCR)) > err = device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default); > } > + > +#ifdef CONFIG_KVMPPC_ENABLE_SECONDARY > +#define NR_CORES (CONFIG_NR_CPUS/threads_per_core) > +static DECLARE_BITMAP(kvm_on_core, NR_CORES) __read_mostly > + > +static ssize_t show_kvm_enable(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > +} > + > +static ssize_t __used store_kvm_enable(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t count) > +{ > + struct cpumask stop_cpus; > + unsigned long core, thr; > + > + sscanf(buf, "%lx", &core); > + if (core > NR_CORES) > + return -1; > + if (!test_bit(core, &kvm_on_core)) > + for (thr = 1; thr< threads_per_core; thr++) > + if (cpu_online(thr * threads_per_core + thr)) > + cpumask_set_cpu(thr * threads_per_core + thr, &stop_cpus); What is the above logic trying to do? Did you mean cpu_online(threads_per_core * core + thr) ? > + > + stop_machine(xics_migrate_irqs_away_secondary, NULL, &stop_cpus); > + set_bit(core, &kvm_on_core); > + return count; > +} > + > +static DEVICE_ATTR(kvm_enable, 0600, > + show_kvm_enable, store_kvm_enable); > + > +static void sysfs_create_kvm_enable(void) > +{ > + device_create_file(cpu_subsys.dev_root, &dev_attr_kvm_enable); > +} > +#endif > + > #endif /* CONFIG_PPC64 */ > > #ifdef HAS_PPC_PMC_PA6T > diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c > index fe0cca4..68b33d8 100644 > --- a/arch/powerpc/sysdev/xics/xics-common.c > +++ b/arch/powerpc/sysdev/xics/xics-common.c > @@ -258,6 +258,18 @@ unlock: > raw_spin_unlock_irqrestore(&desc->lock, flags); > } > } > + > +int xics_migrate_irqs_away_secondary(void *data) > +{ > + int cpu = smp_processor_id(); > + if(cpu%thread_per_core != 0) { > + WARN(condition, format...); > + return 0; > + } > + /* In fact, if we can migrate the primary, it will be more fine */ > + xics_migrate_irqs_away(); Isn't the aim of the patch to migrate irqs away from the secondary onto the primary? But from above it looks like we are returning when we find out that we are secondary threads, isn't it? > + return 0; > +} > #endif /* CONFIG_HOTPLUG_CPU */ Note that xics_migrate_irqs_away() is defined under CONFIG_CPU_HOTPLUG. But we will need this option on PowerKVM even when hotplug is not configured in. Regards Preeti U Murthy > #ifdef CONFIG_SMP >
* kernelfans@gmail.com <kernelfans@gmail.com> [2014-10-16 15:29:52]: > When kvm is enabled on a core, we migrate all external irq to primary > thread. Since currently, the kvmirq logic is handled by the primary > hwthread. > > Todo: this patch lacks re-enable of irqbalance when kvm is disable on > the core > > Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/sysfs.c | 39 ++++++++++++++++++++++++++++++++++ > arch/powerpc/sysdev/xics/xics-common.c | 12 +++++++++++ > 2 files changed, 51 insertions(+) > > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c > index 67fd2fd..a2595dd 100644 > --- a/arch/powerpc/kernel/sysfs.c > +++ b/arch/powerpc/kernel/sysfs.c > @@ -552,6 +552,45 @@ static void sysfs_create_dscr_default(void) > if (cpu_has_feature(CPU_FTR_DSCR)) > err = device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default); > } > + > +#ifdef CONFIG_KVMPPC_ENABLE_SECONDARY > +#define NR_CORES (CONFIG_NR_CPUS/threads_per_core) > +static DECLARE_BITMAP(kvm_on_core, NR_CORES) __read_mostly > + > +static ssize_t show_kvm_enable(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > +} > + > +static ssize_t __used store_kvm_enable(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t count) > +{ > + struct cpumask stop_cpus; > + unsigned long core, thr; > + > + sscanf(buf, "%lx", &core); > + if (core > NR_CORES) > + return -1; > + if (!test_bit(core, &kvm_on_core)) > + for (thr = 1; thr< threads_per_core; thr++) > + if (cpu_online(thr * threads_per_core + thr)) > + cpumask_set_cpu(thr * threads_per_core + thr, &stop_cpus); Shouldnt this be if (cpu_online(core * threads_per_core + thr)) cpumask_set_cpu(core * threads_per_core + thr, &stop_cpus); ?
On Mon, Oct 27, 2014 at 12:04 PM, Preeti U Murthy <preeti@linux.vnet.ibm.com> wrote: > Hi Liu, > > On 10/17/2014 12:59 AM, kernelfans@gmail.com wrote: >> When kvm is enabled on a core, we migrate all external irq to primary >> thread. Since currently, the kvmirq logic is handled by the primary >> hwthread. >> >> Todo: this patch lacks re-enable of irqbalance when kvm is disable on >> the core > > Why is a sysfs file introduced to trigger irq migration? Why is it not > done during kvm module insert ? And similarly spread interrupts when the > module is removed? Isn't this a saner way ? Consider the scene: coreA and coreB, we want to enable KVM on coreA, while keeping coreB unchanged. In fact, I try to acheive something un-symmetric on the platform. Do you think it is an justification? >> >> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> --- >> arch/powerpc/kernel/sysfs.c | 39 ++++++++++++++++++++++++++++++++++ >> arch/powerpc/sysdev/xics/xics-common.c | 12 +++++++++++ >> 2 files changed, 51 insertions(+) >> >> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c >> index 67fd2fd..a2595dd 100644 >> --- a/arch/powerpc/kernel/sysfs.c >> +++ b/arch/powerpc/kernel/sysfs.c >> @@ -552,6 +552,45 @@ static void sysfs_create_dscr_default(void) >> if (cpu_has_feature(CPU_FTR_DSCR)) >> err = device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default); >> } >> + >> +#ifdef CONFIG_KVMPPC_ENABLE_SECONDARY >> +#define NR_CORES (CONFIG_NR_CPUS/threads_per_core) >> +static DECLARE_BITMAP(kvm_on_core, NR_CORES) __read_mostly >> + >> +static ssize_t show_kvm_enable(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> +} >> + >> +static ssize_t __used store_kvm_enable(struct device *dev, >> + struct device_attribute *attr, const char *buf, >> + size_t count) >> +{ >> + struct cpumask stop_cpus; >> + unsigned long core, thr; >> + >> + sscanf(buf, "%lx", &core); >> + if (core > NR_CORES) >> + return -1; >> + if (!test_bit(core, &kvm_on_core)) >> + for (thr = 1; thr< threads_per_core; thr++) >> + if (cpu_online(thr * threads_per_core + thr)) >> + cpumask_set_cpu(thr * threads_per_core + thr, &stop_cpus); > > What is the above logic trying to do? Did you mean > cpu_online(threads_per_core * core + thr) ? > Yeah. My mistake, should be cpumask_set_cpu(core * threads_per_core + thr, &stop_cpus) >> + >> + stop_machine(xics_migrate_irqs_away_secondary, NULL, &stop_cpus); >> + set_bit(core, &kvm_on_core); >> + return count; >> +} >> + >> +static DEVICE_ATTR(kvm_enable, 0600, >> + show_kvm_enable, store_kvm_enable); >> + >> +static void sysfs_create_kvm_enable(void) >> +{ >> + device_create_file(cpu_subsys.dev_root, &dev_attr_kvm_enable); >> +} >> +#endif >> + >> #endif /* CONFIG_PPC64 */ >> >> #ifdef HAS_PPC_PMC_PA6T >> diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c >> index fe0cca4..68b33d8 100644 >> --- a/arch/powerpc/sysdev/xics/xics-common.c >> +++ b/arch/powerpc/sysdev/xics/xics-common.c >> @@ -258,6 +258,18 @@ unlock: >> raw_spin_unlock_irqrestore(&desc->lock, flags); >> } >> } >> + >> +int xics_migrate_irqs_away_secondary(void *data) >> +{ >> + int cpu = smp_processor_id(); >> + if(cpu%thread_per_core != 0) { >> + WARN(condition, format...); >> + return 0; >> + } >> + /* In fact, if we can migrate the primary, it will be more fine */ >> + (); > > Isn't the aim of the patch to migrate irqs away from the secondary onto > the primary? But from above it looks like we are returning when we find > out that we are secondary threads, isn't it? > Yes, will fix in next version. >> + return 0; >> +} >> #endif /* CONFIG_HOTPLUG_CPU */ > > Note that xics_migrate_irqs_away() is defined under CONFIG_CPU_HOTPLUG. > But we will need this option on PowerKVM even when hotplug is not > configured in. > Yes, will fix the dependency in next version Thx, Fan > Regards > Preeti U Murthy >> #ifdef CONFIG_SMP >> >
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c index 67fd2fd..a2595dd 100644 --- a/arch/powerpc/kernel/sysfs.c +++ b/arch/powerpc/kernel/sysfs.c @@ -552,6 +552,45 @@ static void sysfs_create_dscr_default(void) if (cpu_has_feature(CPU_FTR_DSCR)) err = device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default); } + +#ifdef CONFIG_KVMPPC_ENABLE_SECONDARY +#define NR_CORES (CONFIG_NR_CPUS/threads_per_core) +static DECLARE_BITMAP(kvm_on_core, NR_CORES) __read_mostly + +static ssize_t show_kvm_enable(struct device *dev, + struct device_attribute *attr, char *buf) +{ +} + +static ssize_t __used store_kvm_enable(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + struct cpumask stop_cpus; + unsigned long core, thr; + + sscanf(buf, "%lx", &core); + if (core > NR_CORES) + return -1; + if (!test_bit(core, &kvm_on_core)) + for (thr = 1; thr< threads_per_core; thr++) + if (cpu_online(thr * threads_per_core + thr)) + cpumask_set_cpu(thr * threads_per_core + thr, &stop_cpus); + + stop_machine(xics_migrate_irqs_away_secondary, NULL, &stop_cpus); + set_bit(core, &kvm_on_core); + return count; +} + +static DEVICE_ATTR(kvm_enable, 0600, + show_kvm_enable, store_kvm_enable); + +static void sysfs_create_kvm_enable(void) +{ + device_create_file(cpu_subsys.dev_root, &dev_attr_kvm_enable); +} +#endif + #endif /* CONFIG_PPC64 */ #ifdef HAS_PPC_PMC_PA6T diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c index fe0cca4..68b33d8 100644 --- a/arch/powerpc/sysdev/xics/xics-common.c +++ b/arch/powerpc/sysdev/xics/xics-common.c @@ -258,6 +258,18 @@ unlock: raw_spin_unlock_irqrestore(&desc->lock, flags); } } + +int xics_migrate_irqs_away_secondary(void *data) +{ + int cpu = smp_processor_id(); + if(cpu%thread_per_core != 0) { + WARN(condition, format...); + return 0; + } + /* In fact, if we can migrate the primary, it will be more fine */ + xics_migrate_irqs_away(); + return 0; +} #endif /* CONFIG_HOTPLUG_CPU */ #ifdef CONFIG_SMP
When kvm is enabled on a core, we migrate all external irq to primary thread. Since currently, the kvmirq logic is handled by the primary hwthread. Todo: this patch lacks re-enable of irqbalance when kvm is disable on the core Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> --- arch/powerpc/kernel/sysfs.c | 39 ++++++++++++++++++++++++++++++++++ arch/powerpc/sysdev/xics/xics-common.c | 12 +++++++++++ 2 files changed, 51 insertions(+)