[PATCH] x86/irq: Do not check available vectors if current CPU has no irq to migrate

[PATCH] x86/irq: Do not check available vectors if current CPU has no irq to migrate

[Chen Yu]

This is an optimization to bypass the free vector checking if the current
CPU has no irq to migrate. This can especially speed up the CPU offline
process when there are many CPUs in the system.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 arch/x86/kernel/irq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 4d8183b..b9bdc1e 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -394,6 +394,9 @@ int check_irq_vectors_for_cpu_disable(void)
 		    !cpumask_subset(&affinity_new, &online_new))
 			this_count++;
 	}
+	/* No need to check any further. */
+	if (!this_count)
+		return 0;
 
 	count = 0;
 	for_each_online_cpu(cpu) {
-- 
2.7.4

Re: [PATCH] x86/irq: Do not check available vectors if current CPU has no irq to migrate

[Prarit Bhargava]

On 04/09/2017 09:53 AM, Chen Yu wrote:
> This is an optimization to bypass the free vector checking if the current
> CPU has no irq to migrate. This can especially speed up the CPU offline
> process when there are many CPUs in the system.

Chen, OOC how much time does this save?

P.

> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Len Brown <len.brown@intel.com>
> Cc: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  arch/x86/kernel/irq.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index 4d8183b..b9bdc1e 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -394,6 +394,9 @@ int check_irq_vectors_for_cpu_disable(void)
>  		    !cpumask_subset(&affinity_new, &online_new))
>  			this_count++;
>  	}
> +	/* No need to check any further. */
> +	if (!this_count)
> +		return 0;
>  
>  	count = 0;
>  	for_each_online_cpu(cpu) {
> 

Re: [PATCH] x86/irq: Do not check available vectors if current CPU has no irq to migrate

[Chen Yu]

Hi Prarit,
On Mon, Apr 10, 2017 at 07:26:31AM -0400, Prarit Bhargava wrote:
> 
> 
> On 04/09/2017 09:53 AM, Chen Yu wrote:
> > This is an optimization to bypass the free vector checking if the current
> > CPU has no irq to migrate. This can especially speed up the CPU offline
> > process when there are many CPUs in the system.
> 
> Chen, OOC how much time does this save?
> 
Previously I was doing some debugging on irq related issue
when check_irq_vectors_for_cpu_disable() printed a warning
on that platform, thus I incidentally look at this code and
did not measure it : )

I've just done a comparison of the function duration of
check_irq_vectors_for_cpu_disable() with/without the patch
(32 online CPUs)
before patch:
[001] d..1   264.017803: check_irq_vectors_for_cpu_disable <-native_cpu_disable
[001] d..1   264.017886: clear_local_APIC <-native_cpu_disable
after patch:
[001] d..1   862.011847: check_irq_vectors_for_cpu_disable <-native_cpu_disable
[001] d..1   862.011850: clear_local_APIC <-native_cpu_disable
About  (83 - 17) = 66 microsecond.

thanks,
Yu
> P.
> 
> > 
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: Prarit Bhargava <prarit@redhat.com>
> > Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > Cc: Len Brown <len.brown@intel.com>
> > Cc: x86@kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> >  arch/x86/kernel/irq.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> > index 4d8183b..b9bdc1e 100644
> > --- a/arch/x86/kernel/irq.c
> > +++ b/arch/x86/kernel/irq.c
> > @@ -394,6 +394,9 @@ int check_irq_vectors_for_cpu_disable(void)
> >  		    !cpumask_subset(&affinity_new, &online_new))
> >  			this_count++;
> >  	}
> > +	/* No need to check any further. */
> > +	if (!this_count)
> > +		return 0;
> >  
> >  	count = 0;
> >  	for_each_online_cpu(cpu) {
> > 

Re: [PATCH] x86/irq: Do not check available vectors if current CPU has no irq to migrate

[Thomas Gleixner]

On Mon, 10 Apr 2017, Chen Yu wrote:
> I've just done a comparison of the function duration of
> check_irq_vectors_for_cpu_disable() with/without the patch
> (32 online CPUs)
> before patch:
> [001] d..1   264.017803: check_irq_vectors_for_cpu_disable <-native_cpu_disable
> [001] d..1   264.017886: clear_local_APIC <-native_cpu_disable
> after patch:
> [001] d..1   862.011847: check_irq_vectors_for_cpu_disable <-native_cpu_disable
> [001] d..1   862.011850: clear_local_APIC <-native_cpu_disable
> About  (83 - 17) = 66 microsecond.

Not really impressive if you subtract the tracer overhead. Though, if you
want to micro optimize that, then you should do it proper and also break
the for_each_cpu_loop() when the count reaches the required vectors.

Thanks,

	tglx
	
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -411,8 +411,10 @@ int check_irq_vectors_for_cpu_disable(vo
 		for (vector = FIRST_EXTERNAL_VECTOR;
 		     vector < first_system_vector; vector++) {
 			if (!test_bit(vector, used_vectors) &&
-			    IS_ERR_OR_NULL(per_cpu(vector_irq, cpu)[vector]))
-			    count++;
+			    IS_ERR_OR_NULL(per_cpu(vector_irq, cpu)[vector])) {
+				if (++count == this_count)
+					return 0;
+			}
 		}
 	}