[PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]

in phys flat mode, when using set_xxx_irq_affinity to irq balance from
one cpu to another,  _assign_irq_vector will get to increase last used
vector and get new vector. this will use up the vector if enough
set_xxx_irq_affintiy are called. and end with using same vector in
different cpu for different irq. (that is not what we want, we only
want to use same vector in different cpu for different irq when more
than 0x240 irq needed). To keep it simple, the vector should be resued
from one cpu to another instead of getting new vector.

Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>

[-- Attachment #2: io_apic_c_reuse_vector.diff --]
[-- Type: text/x-patch, Size: 1191 bytes --]

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index b000017..3989fa5 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -624,11 +624,32 @@ static int __assign_irq_vector(int irq, 
 	if (irq_vector[irq] > 0)
 		old_vector = irq_vector[irq];
 	if (old_vector > 0) {
+		cpumask_t domain, new_mask, old_mask;
+		int new_cpu, old_cpu;
 		cpus_and(*result, irq_domain[irq], mask);
 		if (!cpus_empty(*result))
 			return old_vector;
+
+		/* try to reuse vector for phys flat */
+		domain = vector_allocation_domain(cpu);
+		cpus_and(new_mask, domain, cpu_online_map);
+		for_each_cpu_mask(new_cpu, new_mask)
+			if (per_cpu(vector_irq, new_cpu)[old_vector] != -1)
+				goto new_vector;
+		/* We can reuse it */	
+		cpus_and(old_mask, irq_domain[irq], cpu_online_map);
+		for_each_cpu_mask(old_cpu, old_mask);
+			per_cpu(vector_irq, old_cpu)[old_vector] = -1;
+		for_each_cpu_mask(new_cpu, new_mask)
+			per_cpu(vector_irq, new_cpu)[old_vector] = irq;
+		irq_domain[irq] = domain;
+		cpus_and(*result, domain, mask);
+		return old_vector;
+		
 	}
 
+new_vector:
+
 	for_each_cpu_mask(cpu, mask) {
 		cpumask_t domain;
 		int first, new_cpu;

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Muli Ben-Yehuda]

On Mon, Oct 23, 2006 at 12:02:44AM -0700, Yinghai Lu wrote:
> in phys flat mode, when using set_xxx_irq_affinity to irq balance from
> one cpu to another,  _assign_irq_vector will get to increase last used
> vector and get new vector. this will use up the vector if enough
> set_xxx_irq_affintiy are called. and end with using same vector in
> different cpu for different irq. (that is not what we want, we only
> want to use same vector in different cpu for different irq when more
> than 0x240 irq needed). To keep it simple, the vector should be resued
> from one cpu to another instead of getting new vector.
> 
> Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>

Should I give this a spin? with or without Eric's two patches?

Cheers,
Muli

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Eric W. Biederman]

"Yinghai Lu" <yinghai.lu@amd.com> writes:

> in phys flat mode, when using set_xxx_irq_affinity to irq balance from
> one cpu to another,  _assign_irq_vector will get to increase last used
> vector and get new vector. this will use up the vector if enough
> set_xxx_irq_affintiy are called. and end with using same vector in
> different cpu for different irq. (that is not what we want, we only
> want to use same vector in different cpu for different irq when more
> than 0x240 irq needed). To keep it simple, the vector should be resued
> from one cpu to another instead of getting new vector.
>
> Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>

YH.  I think the concept is sound.  I don't think this is a bug fix, just
an optimization so this may not be 2.6.19 material.  But we are thrashing
things so much it may make sense to include it, and it likely to keeps
us from running into problems, so it can be called a bug preventative :)

Beyond that I have a few nits to pick with the patch.
- We duplicate the code that claims a new vector which makes
  maintenance a pain.
- The comments are specific to phys_flat but the code is not.
- The test for being able to use the old_vector in the new domain
  should be: ...[old_vector] == vector || ...[old_vector] == -1

Eric

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[yhlu]

On 10/23/06, Muli Ben-Yehuda <muli@il.ibm.com> wrote:
> Should I give this a spin? with or without Eric's two patches?
>
should be with Eric's patch with cpu_online... consistent.

It should be helpful to phys_flat. and that code should be reached by
flat (logical) mode.

YH

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Muli Ben-Yehuda]

On Mon, Oct 23, 2006 at 12:02:44AM -0700, Yinghai Lu wrote:
> in phys flat mode, when using set_xxx_irq_affinity to irq balance from
> one cpu to another,  _assign_irq_vector will get to increase last used
> vector and get new vector. this will use up the vector if enough
> set_xxx_irq_affintiy are called. and end with using same vector in
> different cpu for different irq. (that is not what we want, we only
> want to use same vector in different cpu for different irq when more
> than 0x240 irq needed). To keep it simple, the vector should be resued
> from one cpu to another instead of getting new vector.
> 
> Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>

> diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
> index b000017..3989fa5 100644
> --- a/arch/x86_64/kernel/io_apic.c
> +++ b/arch/x86_64/kernel/io_apic.c
> @@ -624,11 +624,32 @@ static int __assign_irq_vector(int irq, 
>  	if (irq_vector[irq] > 0)
>  		old_vector = irq_vector[irq];
>  	if (old_vector > 0) {
> +		cpumask_t domain, new_mask, old_mask;
> +		int new_cpu, old_cpu;
>  		cpus_and(*result, irq_domain[irq], mask);
>  		if (!cpus_empty(*result))
>  			return old_vector;
> +
> +		/* try to reuse vector for phys flat */
> +		domain = vector_allocation_domain(cpu);

cpu is used unitialized here. Please send an updated patch and I'll
give it a spin.

Cheers,
Muli

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Adrian Bunk]

On Mon, Oct 23, 2006 at 09:32:57AM -0600, Eric W. Biederman wrote:
> "Yinghai Lu" <yinghai.lu@amd.com> writes:
> 
> > in phys flat mode, when using set_xxx_irq_affinity to irq balance from
> > one cpu to another,  _assign_irq_vector will get to increase last used
> > vector and get new vector. this will use up the vector if enough
> > set_xxx_irq_affintiy are called. and end with using same vector in
> > different cpu for different irq. (that is not what we want, we only
> > want to use same vector in different cpu for different irq when more
> > than 0x240 irq needed). To keep it simple, the vector should be resued
> > from one cpu to another instead of getting new vector.
> >
> > Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>
> 
> YH.  I think the concept is sound.  I don't think this is a bug fix, just
> an optimization so this may not be 2.6.19 material.  But we are thrashing
> things so much it may make sense to include it, and it likely to keeps
> us from running into problems, so it can be called a bug preventative :)
>...

Is this patch intended as fix for the 2.6.19-rc regression described 
in [1]?

> Eric

cu
Adrian

[1] http://lkml.org/lkml/2006/10/21/227

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Muli Ben-Yehuda]

On Mon, Oct 23, 2006 at 10:45:19PM +0200, Adrian Bunk wrote:
> On Mon, Oct 23, 2006 at 09:32:57AM -0600, Eric W. Biederman wrote:
> > "Yinghai Lu" <yinghai.lu@amd.com> writes:
> > 
> > > in phys flat mode, when using set_xxx_irq_affinity to irq balance from
> > > one cpu to another,  _assign_irq_vector will get to increase last used
> > > vector and get new vector. this will use up the vector if enough
> > > set_xxx_irq_affintiy are called. and end with using same vector in
> > > different cpu for different irq. (that is not what we want, we only
> > > want to use same vector in different cpu for different irq when more
> > > than 0x240 irq needed). To keep it simple, the vector should be resued
> > > from one cpu to another instead of getting new vector.
> > >
> > > Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>
> > 
> > YH.  I think the concept is sound.  I don't think this is a bug fix, just
> > an optimization so this may not be 2.6.19 material.  But we are thrashing
> > things so much it may make sense to include it, and it likely to keeps
> > us from running into problems, so it can be called a bug preventative :)
> >...
> 
> Is this patch intended as fix for the 2.6.19-rc regression described 
> in [1]?

No, that regression is fixed by the two x86-64 patches Eric sent in
http://marc.theaimsgroup.com/?t=116157840300001&r=1&w=2.

Cheers,
Muli

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[yhlu]

On 10/23/06, Oleg Verych <olecom@flower.upol.cz> wrote:
> Hallo, Lu.
>
> On 2006-10-23, Lu, Yinghai wrote:
> > This is a multi-part message in MIME format.

I have to use outlook in my office to send email. If i use attachment,
that is what you get.

If i cut and paste, I will get wordwrapped.

I have to use gmail to send out patch...

Please check another thread for the patch.

http://lkml.org/lkml/2006/10/24/1

YH

Re: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Oleg Verych]

Hallo, Lu.

On 2006-10-23, Lu, Yinghai wrote:
> This is a multi-part message in MIME format.
[]
> ------_=_NextPart_001_01C6F6DD.E040B4D2
> Content-Type: application/octet-stream;
>  name=io_apic_reuse_vector.diff
> Content-Transfer-Encoding: base64
> Content-Description: io_apic_reuse_vector.diff
> Content-Disposition: attachment;
>  filename=io_apic_reuse_vector.diff
>
> LS0tIGFyY2gveDg2XzY0L2tlcm5lbC9pb19hcGljX2VyaWMuYwkyMDA2LTEwLTIzIDExOjU2OjM2
> LjAwMDAwMDAwMCAtMDcwMAorKysgYXJjaC94ODZfNjQva2VybmVsL2lvX2FwaWMuYwkyMDA2LTEw

What's problem with Documentation/SubmittingPatches.6 ? 
,-
|6) No MIME, no links, no compression, no attachments.  Just plain text.
`-

> LTIzIDExOjIyOjMxLjAwMDAwMDAwMCAtMDcwMApAQCAtNjEzLDggKzYxMyw5IEBACiAJICogMHg4
> MCwgYmVjYXVzZSBpbnQgMHg4MCBpcyBobSwga2luZCBvZiBpbXBvcnRhbnRpc2guIDspCiAJICov
[]
____

RE: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Lu, Yinghai]

>From: Adrian Bunk [mailto:bunk@stusta.de] 
>Sent: Monday, October 23, 2006 1:45 PM

>Is this patch intended as fix for the 2.6.19-rc regression described 
>in [1]?


>[1] http://lkml.org/lkml/2006/10/21/227

This patch is intended to reuse vector between cpus in phys_flat when
using irq balance to different cpus.

YH

RE: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Lu, Yinghai]

resend


>From: ebiederm@xmission.com [mailto:ebiederm@xmission.com] 

>Beyond that I have a few nits to pick with the patch.
>- We duplicate the code that claims a new vector which makes
>  maintenance a pain.
>- The comments are specific to phys_flat but the code is not.
>- The test for being able to use the old_vector in the new domain
>  should be: ...[old_vector] == vector || ...[old_vector] == -1

Please attached one. This one need Eric's patch about irq global vector.

YH


--- linux-2.6/arch/x86_64/kernel/io_apic_eric.c	2006-10-23
11:56:36.000000000 -0700
+++ linux-2.6/arch/x86_64/kernel/io_apic.c	2006-10-23
13:59:36.000000000 -0700
@@ -613,8 +613,9 @@
 	 * 0x80, because int 0x80 is hm, kind of importantish. ;)
 	 */
 	static int current_vector = FIRST_DEVICE_VECTOR, current_offset
= 0;
-	int old_vector = -1;
-	int cpu;
+	int vector = -1, old_vector = -1;
+	cpumask_t domain, new_mask;
+	int cpu, new_cpu;
 
 	BUG_ON((unsigned)irq >= NR_IRQ_VECTORS);
 
@@ -628,12 +629,30 @@
 		if (!cpus_empty(*result))
 			return old_vector;
 
+		/* try to reuse vector */
+		for_each_cpu_mask(cpu, mask) {
+			int can_resue = 1;
+			domain = vector_allocation_domain(cpu);
+			cpus_and(new_mask, domain, cpu_online_map);
+			for_each_cpu_mask(new_cpu, new_mask) {
+				int old_irq;
+				old_irq = per_cpu(vector_irq,
new_cpu)[old_vector];
+				if ( (old_irq != irq) && (old_irq !=
-1)) {
+					can_resue = 0;
+					break;
+				}
+			}
+
+			if(!can_resue) continue;
+
+			vector = old_vector;
+			goto found_one;	
+		}
+
 	}
 
 	for_each_cpu_mask(cpu, mask) {
-		cpumask_t domain, new_mask;
-		int new_cpu;
-		int vector, offset;
+		int offset;
 
 		domain = vector_allocation_domain(cpu);
 		cpus_and(new_mask, domain, cpu_online_map);
@@ -657,21 +676,27 @@
 		/* Found one! */
 		current_vector = vector;
 		current_offset = offset;
-		if (old_vector >= 0) {
-			cpumask_t old_mask;
-			int old_cpu;
-			cpus_and(old_mask, irq_domain[irq],
cpu_online_map);
-			for_each_cpu_mask(old_cpu, old_mask)
-				per_cpu(vector_irq, old_cpu)[old_vector]
= -1;
-		}
-		for_each_cpu_mask(new_cpu, new_mask)
-			per_cpu(vector_irq, new_cpu)[vector] = irq;
-		irq_vector[irq] = vector;
-		irq_domain[irq] = domain;
-		cpus_and(*result, domain, mask);
-		return vector;
+		
+		goto found_one;
 	}
+
 	return -ENOSPC;
+
+found_one:
+	if (old_vector >= 0) {
+		cpumask_t old_mask;
+		int old_cpu;
+		cpus_and(old_mask, irq_domain[irq], cpu_online_map);
+		for_each_cpu_mask(old_cpu, old_mask)
+			per_cpu(vector_irq, old_cpu)[old_vector] = -1;
+	}
+	for_each_cpu_mask(new_cpu, new_mask)
+		per_cpu(vector_irq, new_cpu)[vector] = irq;
+	irq_vector[irq] = vector;
+	irq_domain[irq] = domain;
+	cpus_and(*result, domain, mask);
+	return vector;
+
 }
 
 static int assign_irq_vector(int irq, cpumask_t mask, cpumask_t
*result)

RE: [PATCH] x86_64 irq: reuse vector for set_xxx_irq_affinity in phys flat mode

[Lu, Yinghai]

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

From: ebiederm@xmission.com [mailto:ebiederm@xmission.com] 

>Beyond that I have a few nits to pick with the patch.
>- We duplicate the code that claims a new vector which makes
>  maintenance a pain.
>- The comments are specific to phys_flat but the code is not.
>- The test for being able to use the old_vector in the new domain
>  should be: ...[old_vector] == vector || ...[old_vector] == -1

Please attached one. This one need to after your patch about irq.

YH


[-- Attachment #2: io_apic_reuse_vector.diff --]
[-- Type: application/octet-stream, Size: 2306 bytes --]

--- arch/x86_64/kernel/io_apic_eric.c	2006-10-23 11:56:36.000000000 -0700
+++ arch/x86_64/kernel/io_apic.c	2006-10-23 11:22:31.000000000 -0700
@@ -613,8 +613,9 @@
 	 * 0x80, because int 0x80 is hm, kind of importantish. ;)
 	 */
 	static int current_vector = FIRST_DEVICE_VECTOR, current_offset = 0;
-	int old_vector = -1;
-	int cpu;
+	int vector = -1, old_vector = -1;
+	cpumask_t domain, new_mask;
+	int cpu, new_cpu;
 
 	BUG_ON((unsigned)irq >= NR_IRQ_VECTORS);
 
@@ -628,12 +629,29 @@
 		if (!cpus_empty(*result))
 			return old_vector;
 
+		/* try to reuse vector */
+		for_each_cpu_mask(cpu, mask) {
+			int can_resue = 1;
+			domain = vector_allocation_domain(cpu);
+			cpus_and(new_mask, domain, cpu_online_map);
+			for_each_cpu_mask(new_cpu, new_mask) {
+				int old_irq;
+				old_irq = per_cpu(vector_irq, new_cpu)[old_vector];
+				if ( (old_irq != irq) && (old_irq != -1)) {
+					can_resue = 0;
+					break;
+				}
+			}
+
+			if(!can_resue) continue;
+
+			vector = old_vector;
+			goto found_one;	
+		}
 	}
 
 	for_each_cpu_mask(cpu, mask) {
-		cpumask_t domain, new_mask;
-		int new_cpu;
-		int vector, offset;
+		int offset;
 
 		domain = vector_allocation_domain(cpu);
 		cpus_and(new_mask, domain, cpu_online_map);
@@ -657,21 +675,27 @@
 		/* Found one! */
 		current_vector = vector;
 		current_offset = offset;
-		if (old_vector >= 0) {
-			cpumask_t old_mask;
-			int old_cpu;
-			cpus_and(old_mask, irq_domain[irq], cpu_online_map);
-			for_each_cpu_mask(old_cpu, old_mask)
-				per_cpu(vector_irq, old_cpu)[old_vector] = -1;
-		}
-		for_each_cpu_mask(new_cpu, new_mask)
-			per_cpu(vector_irq, new_cpu)[vector] = irq;
-		irq_vector[irq] = vector;
-		irq_domain[irq] = domain;
-		cpus_and(*result, domain, mask);
-		return vector;
+		
+		goto found_one;
 	}
+
 	return -ENOSPC;
+
+found_one:
+	if (old_vector >= 0) {
+		cpumask_t old_mask;
+		int old_cpu;
+		cpus_and(old_mask, irq_domain[irq], cpu_online_map);
+		for_each_cpu_mask(old_cpu, old_mask)
+			per_cpu(vector_irq, old_cpu)[old_vector] = -1;
+	}
+	for_each_cpu_mask(new_cpu, new_mask)
+		per_cpu(vector_irq, new_cpu)[vector] = irq;
+	irq_vector[irq] = vector;
+	irq_domain[irq] = domain;
+	cpus_and(*result, domain, mask);
+	return vector;
+
 }
 
 static int assign_irq_vector(int irq, cpumask_t mask, cpumask_t *result)