2006-10-24 21:33:27

by Lu, Yinghai

[permalink] [raw]
Subject: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq


Clear the irq releated entries in irq_vector, irq_domain and vector_irq
instead of clearing irq_vector only. So when new irq is created, it
could get that vector.

Signed-off-By: Yinghai Lu <[email protected]>

--- linux-2.6/arch/x86_64/kernel/io_apic.c 2006-10-24
13:40:48.000000000 -0700
+++ linux-2.6.xx/arch/x86_64/kernel/io_apic.c 2006-10-24
14:03:08.000000000 -0700
@@ -716,6 +716,22 @@
return vector;
}

+static void __clear_irq_vector(int irq)
+{
+ int old_vector = -1;
+ if (irq_vector[irq] > 0)
+ old_vector = irq_vector[irq];
+ 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;
+ }
+ irq_vector[irq] = 0;
+ irq_domain[irq] = CPU_MASK_NONE;
+}
+
void __setup_vector_irq(int cpu)
{
/* Initialize vector_irq on a new cpu */
@@ -1803,7 +1819,7 @@
dynamic_irq_cleanup(irq);

spin_lock_irqsave(&vector_lock, flags);
- irq_vector[irq] = 0;
+ __clear_irq_vector(irq);
spin_unlock_irqrestore(&vector_lock, flags);
}




2006-10-24 22:53:40

by Josef Sipek

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

On Tue, Oct 24, 2006 at 02:33:08PM -0700, Lu, Yinghai wrote:
>
> Clear the irq releated entries in irq_vector, irq_domain and vector_irq
> instead of clearing irq_vector only. So when new irq is created, it
> could get that vector.
>
> Signed-off-By: Yinghai Lu <[email protected]>
>
> --- linux-2.6/arch/x86_64/kernel/io_apic.c 2006-10-24
> 13:40:48.000000000 -0700
> +++ linux-2.6.xx/arch/x86_64/kernel/io_apic.c 2006-10-24
> 14:03:08.000000000 -0700
> @@ -716,6 +716,22 @@

Your patch got mangled up.

Josef "Jeff" Sipek.

--
Linux, n.:
Generous programmers from around the world all join forces to help
you shoot yourself in the foot for free.

2006-10-25 03:46:34

by Lu, Yinghai

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

resend with gmail.

Clear the irq releated entries in irq_vector, irq_domain and vector_irq
instead of clearing irq_vector only. So when new irq is created, it
could reuse that vector. (actually is the second loop scanning from
FIRST_DEVICE_VECTOR+8). This could avoid the vectors are used up
with enough module inserting and removing

Cc: Eric W. Biedierman <[email protected]>
Signed-off-By: Yinghai Lu <[email protected]>


Attachments:
(No filename) (430.00 B)
io_apic_clear_irq_vector.diff (885.00 B)
Download all attachments

2006-10-25 04:02:09

by Lu, Yinghai

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

Andi,

This could get into your tree.

YH

On 10/24/06, Yinghai Lu <[email protected]> wrote:
> resend with gmail.
>
> Clear the irq releated entries in irq_vector, irq_domain and vector_irq
> instead of clearing irq_vector only. So when new irq is created, it
> could reuse that vector. (actually is the second loop scanning from
> FIRST_DEVICE_VECTOR+8). This could avoid the vectors are used up
> with enough module inserting and removing
>
> Cc: Eric W. Biedierman <[email protected]>
> Signed-off-By: Yinghai Lu <[email protected]>
>
>
>

2006-10-25 04:53:04

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

On Tue, 24 Oct 2006, Yinghai Lu wrote:

> --- linux-2.6/arch/x86_64/kernel/io_apic.c 2006-10-24 13:40:48.000000000 -0700
> +++ linux-2.6.xx/arch/x86_64/kernel/io_apic.c 2006-10-24 14:03:08.000000000 -0700
> @@ -716,6 +716,22 @@
> return vector;
> }
>
> +static void __clear_irq_vector(int irq)
> +{
> + int old_vector = -1;
> + if (irq_vector[irq] > 0)
> + old_vector = irq_vector[irq];
> + 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;
> + }
> + irq_vector[irq] = 0;
> + irq_domain[irq] = CPU_MASK_NONE;
> +}
> +

The two conditionals don't complement each other; in fact, only one
conditional is required since the test for old_vector equality to zero
will never be satisfied. Also note that irq_vectors are u8 on x86_64 and
not ints.

static void __clear_irq_vector(int irq)
{
u8 old_vector = irq_vector[irq];
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;
}
irq_vector[irq] = 0;
irq_domain[irq] = CPU_MASK_NONE;
}

2006-10-25 06:01:27

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

I cut the code paste the lines from __assign_irq_vector().

It seems we need to change
if (old_vector >= 0) {
in __assign_irq_vector with this one to
if (old_vector > 0) {
later. Eric?

YH




> The two conditionals don't complement each other; in fact, only one
> conditional is required since the test for old_vector equality to zero
> will never be satisfied. Also note that irq_vectors are u8 on x86_64 and
> not ints.
>
> static void __clear_irq_vector(int irq)
> {
> u8 old_vector = irq_vector[irq];
> 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;
> }
> irq_vector[irq] = 0;
> irq_domain[irq] = CPU_MASK_NONE;
> }
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2006-10-25 11:30:21

by Muli Ben-Yehuda

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

On Tue, Oct 24, 2006 at 08:46:31PM -0700, Yinghai Lu wrote:
> resend with gmail.
>
> Clear the irq releated entries in irq_vector, irq_domain and vector_irq
> instead of clearing irq_vector only. So when new irq is created, it
> could reuse that vector. (actually is the second loop scanning from
> FIRST_DEVICE_VECTOR+8). This could avoid the vectors are used up
> with enough module inserting and removing
>
> Cc: Eric W. Biedierman <[email protected]>
> Signed-off-By: Yinghai Lu <[email protected]>

I hope I'm testing the right patch... this one boots and works fine.

Cheers,
Muli

2006-10-25 18:27:45

by Lu, Yinghai

[permalink] [raw]
Subject: RE: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

Thanks.

I only found ht_irq and msi call destroy_irq. How about io_apic?

YH

-----Original Message-----
From: Muli Ben-Yehuda [mailto:[email protected]]
Sent: Wednesday, October 25, 2006 4:30 AM
To: Lu, Yinghai
Cc: Andi Kleen; Eric W. Biederman; Andrew Morton; Linux Kernel Mailing
List
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear
irq_vector for destroy_irq

On Tue, Oct 24, 2006 at 08:46:31PM -0700, Yinghai Lu wrote:
> resend with gmail.
>
> Clear the irq releated entries in irq_vector, irq_domain and
vector_irq
> instead of clearing irq_vector only. So when new irq is created, it
> could reuse that vector. (actually is the second loop scanning from
> FIRST_DEVICE_VECTOR+8). This could avoid the vectors are used up
> with enough module inserting and removing
>
> Cc: Eric W. Biedierman <[email protected]>
> Signed-off-By: Yinghai Lu <[email protected]>

I hope I'm testing the right patch... this one boots and works fine.

Cheers,
Muli




2006-10-27 06:23:29

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

"Lu, Yinghai" <[email protected]> writes:

> Thanks.
>
> I only found ht_irq and msi call destroy_irq. How about io_apic?

Only on io_apic hot unplug which we don't currently support.

Eric

2006-10-28 05:44:39

by Lu, Yinghai

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

revised version according to Eric. and it can be applied clearly to
current Linus's Tree.

Clear the irq releated entries in irq_vector, irq_domain and vector_irq
instead of clearing irq_vector only. So when new irq is created, it
could reuse that vector. (actually is the second loop scanning from
FIRST_DEVICE_VECTOR+8). This could avoid the vectors are used up
with enough module inserting and removing

Cc: Eric W. Biedierman <[email protected]>
Cc: Muli Ben-Yehuda <[email protected]>
Signed-off-By: Yinghai Lu <[email protected]>


Attachments:
(No filename) (539.00 B)
io_apic_clear_irq_vector_1027.diff (898.00 B)
Download all attachments

2006-10-28 09:19:12

by Oleg Verych

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

Again, mister Yinghai Lu, bother to do patches, but you
"have to use outlook in [] office to send email".?

Anyway, _bother_ apply Documentation/SubmittingPatches.6, *please*.
,-
|6) No MIME, no links, no compression, no attachments. Just plain text.
`-

> ------=_Part_69251_21055565.1162014276979
> Content-Type: text/x-patch; name=io_apic_clear_irq_vector_1027.diff;
> charset=ANSI_X3.4-1968
> Content-Transfer-Encoding: base64
> X-Attachment-Id: f_ettl4wte
> Content-Disposition: attachment; filename="io_apic_clear_irq_vector_1027.diff"
>
> ZGlmZiAtLWdpdCBhL2FyY2gveDg2XzY0L2tlcm5lbC9pb19hcGljLmMgYi9hcmNoL3g4Nl82NC9r
> ZXJuZWwvaW9fYXBpYy5jCmluZGV4IGZlNDI5ZTUuLjk3NjYzNGMgMTAwNjQ0Ci0tLSBhL2FyY2gv
> eDg2XzY0L2tlcm5lbC9pb19hcGljLmMKKysrIGIvYXJjaC94ODZfNjQva2VybmVsL2lvX2FwaWMu
[]
____

2006-10-28 17:29:44

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

On Fri, Oct 27, 2006 at 10:44:36PM -0700, Yinghai Lu wrote:
> revised version according to Eric. and it can be applied clearly to
> current Linus's Tree.
>
> Clear the irq releated entries in irq_vector, irq_domain and vector_irq
> instead of clearing irq_vector only. So when new irq is created, it
> could reuse that vector. (actually is the second loop scanning from
> FIRST_DEVICE_VECTOR+8). This could avoid the vectors are used up
> with enough module inserting and removing

Added thanks.

Does i386 need a similar patch?

-Andi

2006-10-28 20:08:54

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH] x86_64 irq: reset more to default when clear irq_vector for destroy_irq

Andi Kleen <[email protected]> writes:

> On Fri, Oct 27, 2006 at 10:44:36PM -0700, Yinghai Lu wrote:
>> revised version according to Eric. and it can be applied clearly to
>> current Linus's Tree.
>>
>> Clear the irq releated entries in irq_vector, irq_domain and vector_irq
>> instead of clearing irq_vector only. So when new irq is created, it
>> could reuse that vector. (actually is the second loop scanning from
>> FIRST_DEVICE_VECTOR+8). This could avoid the vectors are used up
>> with enough module inserting and removing
>
> Added thanks.
>
> Does i386 need a similar patch?

i386 is good, as it doesn't deal with per cpu vectors, so it doesn't
have the additional data structures.

Eric