LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/1] irq: Maintain user set affinity
@ 2008-11-04 16:31 Kumar Gala
  2008-11-06 21:35 ` Andrew Morton
  2008-11-08 22:12 ` Yinghai Lu
  0 siblings, 2 replies; 8+ messages in thread
From: Kumar Gala @ 2008-11-04 16:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, tglx, Andrew Morton, linuxppc-dev

>From 819ad3ed4660f4238e053728a8b5aa93d22b13d7 Mon Sep 17 00:00:00 2001
From: Kumar Gala <galak@kernel.crashing.org>
Date: Tue, 4 Nov 2008 10:13:53 -0600
Subject: [PATCH 1/1] irq: Maintain user set affinity

This addresses a regression in that if a user specified an affinity
for an interrupt that affinity information would get reset between
a request_irq(), free_irq() and request_irq() for the same irq.

On example of this is bringing up an ethernet interface, taking
it down and bringing it back up again.

Instead of always using the irq_default_affinity we use the
previous setting of the IRQs affinity with the assumption that
at boot the IRQs affinity will be set to irq_default_affinity.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 kernel/irq/manage.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index c498a1b..ea7d11b 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -115,9 +115,12 @@ int irq_select_affinity(unsigned int irq)
 	if (!irq_can_set_affinity(irq))
 		return 0;

-	cpus_and(mask, cpu_online_map, irq_default_affinity);
-
 	desc = irq_to_desc(irq);
+
+	mask = desc->affinity;
+
+	cpus_and(mask, cpu_online_map, mask);
+
 	desc->affinity = mask;
 	desc->chip->set_affinity(irq, mask);

-- 
1.5.5.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] irq: Maintain user set affinity
  2008-11-04 16:31 [PATCH 1/1] irq: Maintain user set affinity Kumar Gala
@ 2008-11-06 21:35 ` Andrew Morton
  2008-11-06 23:23   ` Kumar Gala
  2008-11-08 22:13   ` Yinghai Lu
  2008-11-08 22:12 ` Yinghai Lu
  1 sibling, 2 replies; 8+ messages in thread
From: Andrew Morton @ 2008-11-06 21:35 UTC (permalink / raw)
  To: Kumar Gala; +Cc: torvalds, linux-kernel, tglx, linuxppc-dev

On Tue, 4 Nov 2008 10:31:05 -0600 (CST)
Kumar Gala <galak@kernel.crashing.org> wrote:

> >From 819ad3ed4660f4238e053728a8b5aa93d22b13d7 Mon Sep 17 00:00:00 2001
> From: Kumar Gala <galak@kernel.crashing.org>
> Date: Tue, 4 Nov 2008 10:13:53 -0600
> Subject: [PATCH 1/1] irq: Maintain user set affinity
> 
> This addresses a regression in that if a user specified an affinity
> for an interrupt that affinity information would get reset between
> a request_irq(), free_irq() and request_irq() for the same irq.
> 
> On example of this is bringing up an ethernet interface, taking
> it down and bringing it back up again.
> 
> Instead of always using the irq_default_affinity we use the
> previous setting of the IRQs affinity with the assumption that
> at boot the IRQs affinity will be set to irq_default_affinity.
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>  kernel/irq/manage.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index c498a1b..ea7d11b 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -115,9 +115,12 @@ int irq_select_affinity(unsigned int irq)
>  	if (!irq_can_set_affinity(irq))
>  		return 0;
> 
> -	cpus_and(mask, cpu_online_map, irq_default_affinity);
> -
>  	desc = irq_to_desc(irq);
> +
> +	mask = desc->affinity;
> +
> +	cpus_and(mask, cpu_online_map, mask);
> +
>  	desc->affinity = mask;
>  	desc->chip->set_affinity(irq, mask);
> 

Are you able to identify when this regression occurred?  It isn't
immediately obvious to me.

If the regression is present in 2.6.27 or earlier, do you think we
should backport the fix?  If so, a different patch would be needed due
to the presence of

commit 08678b0841267c1d00d771fe01548d86043d065e
Author: Yinghai Lu <yhlu.kernel@gmail.com>
Date:   Tue Aug 19 20:50:05 2008 -0700

    generic: sparse irqs: use irq_desc() together with dyn_array, instead of irq_desc[]


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] irq: Maintain user set affinity
  2008-11-06 21:35 ` Andrew Morton
@ 2008-11-06 23:23   ` Kumar Gala
  2008-11-08 22:13   ` Yinghai Lu
  1 sibling, 0 replies; 8+ messages in thread
From: Kumar Gala @ 2008-11-06 23:23 UTC (permalink / raw)
  To: Andrew Morton; +Cc: torvalds, linux-kernel, tglx, linuxppc-dev


On Nov 6, 2008, at 3:35 PM, Andrew Morton wrote:

> On Tue, 4 Nov 2008 10:31:05 -0600 (CST)
> Kumar Gala <galak@kernel.crashing.org> wrote:
>
>>> From 819ad3ed4660f4238e053728a8b5aa93d22b13d7 Mon Sep 17 00:00:00  
>>> 2001
>> From: Kumar Gala <galak@kernel.crashing.org>
>> Date: Tue, 4 Nov 2008 10:13:53 -0600
>> Subject: [PATCH 1/1] irq: Maintain user set affinity
>>
>> This addresses a regression in that if a user specified an affinity
>> for an interrupt that affinity information would get reset between
>> a request_irq(), free_irq() and request_irq() for the same irq.
>>
>> On example of this is bringing up an ethernet interface, taking
>> it down and bringing it back up again.
>>
>> Instead of always using the irq_default_affinity we use the
>> previous setting of the IRQs affinity with the assumption that
>> at boot the IRQs affinity will be set to irq_default_affinity.
>>
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> ---
>> kernel/irq/manage.c |    7 +++++--
>> 1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
>> index c498a1b..ea7d11b 100644
>> --- a/kernel/irq/manage.c
>> +++ b/kernel/irq/manage.c
>> @@ -115,9 +115,12 @@ int irq_select_affinity(unsigned int irq)
>> 	if (!irq_can_set_affinity(irq))
>> 		return 0;
>>
>> -	cpus_and(mask, cpu_online_map, irq_default_affinity);
>> -
>> 	desc = irq_to_desc(irq);
>> +
>> +	mask = desc->affinity;
>> +
>> +	cpus_and(mask, cpu_online_map, mask);
>> +
>> 	desc->affinity = mask;
>> 	desc->chip->set_affinity(irq, mask);
>>
>
> Are you able to identify when this regression occurred?  It isn't
> immediately obvious to me.
>
> If the regression is present in 2.6.27 or earlier, do you think we
> should backport the fix?  If so, a different patch would be needed due
> to the presence of
>
> commit 08678b0841267c1d00d771fe01548d86043d065e
> Author: Yinghai Lu <yhlu.kernel@gmail.com>
> Date:   Tue Aug 19 20:50:05 2008 -0700
>
>    generic: sparse irqs: use irq_desc() together with dyn_array,  
> instead of irq_desc[]

It didn't creep in during 2.6.27.  I will back port a patch to  
2.6.27.  Thomas has provided me a better patch that handles a few  
critical issues that mine doesn't.  I'll be testing it this evening  
and will backport to 2.6.27 if it works out.

- k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] irq: Maintain user set affinity
  2008-11-04 16:31 [PATCH 1/1] irq: Maintain user set affinity Kumar Gala
  2008-11-06 21:35 ` Andrew Morton
@ 2008-11-08 22:12 ` Yinghai Lu
  2008-11-08 22:17   ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 8+ messages in thread
From: Yinghai Lu @ 2008-11-08 22:12 UTC (permalink / raw)
  To: Kumar Gala
  Cc: Linus Torvalds, linux-kernel, tglx, Andrew Morton, linuxppc-dev

On Tue, Nov 4, 2008 at 8:31 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
> From 819ad3ed4660f4238e053728a8b5aa93d22b13d7 Mon Sep 17 00:00:00 2001
> From: Kumar Gala <galak@kernel.crashing.org>
> Date: Tue, 4 Nov 2008 10:13:53 -0600
> Subject: [PATCH 1/1] irq: Maintain user set affinity
>
> This addresses a regression in that if a user specified an affinity
> for an interrupt that affinity information would get reset between
> a request_irq(), free_irq() and request_irq() for the same irq.
>
> On example of this is bringing up an ethernet interface, taking
> it down and bringing it back up again.
>
> Instead of always using the irq_default_affinity we use the
> previous setting of the IRQs affinity with the assumption that
> at boot the IRQs affinity will be set to irq_default_affinity.

why ?, the default_affinity is expected behavior...

for example: when MSI is used, second driver could get same irq if
first driver is unloaded.

YH

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] irq: Maintain user set affinity
  2008-11-06 21:35 ` Andrew Morton
  2008-11-06 23:23   ` Kumar Gala
@ 2008-11-08 22:13   ` Yinghai Lu
  1 sibling, 0 replies; 8+ messages in thread
From: Yinghai Lu @ 2008-11-08 22:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Kumar Gala, torvalds, linux-kernel, tglx, linuxppc-dev

On Thu, Nov 6, 2008 at 1:35 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 4 Nov 2008 10:31:05 -0600 (CST)
> Kumar Gala <galak@kernel.crashing.org> wrote:
>
>> >From 819ad3ed4660f4238e053728a8b5aa93d22b13d7 Mon Sep 17 00:00:00 2001
>> From: Kumar Gala <galak@kernel.crashing.org>
>> Date: Tue, 4 Nov 2008 10:13:53 -0600
>> Subject: [PATCH 1/1] irq: Maintain user set affinity
>>
>> This addresses a regression in that if a user specified an affinity
>> for an interrupt that affinity information would get reset between
>> a request_irq(), free_irq() and request_irq() for the same irq.
>>
...
>
> Are you able to identify when this regression occurred?  It isn't
> immediately obvious to me.
>
> If the regression is present in 2.6.27 or earlier, do you think we
> should backport the fix?  If so, a different patch would be needed due
> to the presence of

it seems that is not a regression...

YH

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] irq: Maintain user set affinity
  2008-11-08 22:12 ` Yinghai Lu
@ 2008-11-08 22:17   ` Benjamin Herrenschmidt
  2008-11-09  0:01     ` Yinghai Lu
  0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2008-11-08 22:17 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Kumar Gala, Linus Torvalds, linux-kernel, tglx, Andrew Morton,
	linuxppc-dev

On Sat, 2008-11-08 at 14:12 -0800, Yinghai Lu wrote:
> On Tue, Nov 4, 2008 at 8:31 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
> > From 819ad3ed4660f4238e053728a8b5aa93d22b13d7 Mon Sep 17 00:00:00 2001
> > From: Kumar Gala <galak@kernel.crashing.org>
> > Date: Tue, 4 Nov 2008 10:13:53 -0600
> > Subject: [PATCH 1/1] irq: Maintain user set affinity
> >
> > This addresses a regression in that if a user specified an affinity
> > for an interrupt that affinity information would get reset between
> > a request_irq(), free_irq() and request_irq() for the same irq.
> >
> > On example of this is bringing up an ethernet interface, taking
> > it down and bringing it back up again.
> >
> > Instead of always using the irq_default_affinity we use the
> > previous setting of the IRQs affinity with the assumption that
> > at boot the IRQs affinity will be set to irq_default_affinity.
> 
> why ?, the default_affinity is expected behavior...
> 
> for example: when MSI is used, second driver could get same irq if
> first driver is unloaded.

The default affinity should be set when the irq is allocated / assigned
to a device, not when it's requested.

Ben.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] irq: Maintain user set affinity
  2008-11-08 22:17   ` Benjamin Herrenschmidt
@ 2008-11-09  0:01     ` Yinghai Lu
  2008-11-09  2:17       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 8+ messages in thread
From: Yinghai Lu @ 2008-11-09  0:01 UTC (permalink / raw)
  To: benh
  Cc: Kumar Gala, Linus Torvalds, linux-kernel, tglx, Andrew Morton,
	linuxppc-dev

On Sat, Nov 8, 2008 at 2:17 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Sat, 2008-11-08 at 14:12 -0800, Yinghai Lu wrote:
>> On Tue, Nov 4, 2008 at 8:31 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
>> > From 819ad3ed4660f4238e053728a8b5aa93d22b13d7 Mon Sep 17 00:00:00 2001
>> > From: Kumar Gala <galak@kernel.crashing.org>
>> > Date: Tue, 4 Nov 2008 10:13:53 -0600
>> > Subject: [PATCH 1/1] irq: Maintain user set affinity
>> >
>> > This addresses a regression in that if a user specified an affinity
>> > for an interrupt that affinity information would get reset between
>> > a request_irq(), free_irq() and request_irq() for the same irq.
>> >
>> > On example of this is bringing up an ethernet interface, taking
>> > it down and bringing it back up again.
>> >
>> > Instead of always using the irq_default_affinity we use the
>> > previous setting of the IRQs affinity with the assumption that
>> > at boot the IRQs affinity will be set to irq_default_affinity.
>>
>> why ?, the default_affinity is expected behavior...
>>
>> for example: when MSI is used, second driver could get same irq if
>> first driver is unloaded.
>
> The default affinity should be set when the irq is allocated / assigned
> to a device, not when it's requested.

ok, you are right.

for MSI, affinity is set two times.

also in dynamic_irq_init is setting with
#ifdef CONFIG_SMP
        cpus_setall(desc->affinity);
#endif

it seems should use irq_default_affinity instead of CPU_MASK_ALL directly there.

YH

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] irq: Maintain user set affinity
  2008-11-09  0:01     ` Yinghai Lu
@ 2008-11-09  2:17       ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2008-11-09  2:17 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Kumar Gala, Linus Torvalds, linux-kernel, tglx, Andrew Morton,
	linuxppc-dev


> for MSI, affinity is set two times.
> 
> also in dynamic_irq_init is setting with
> #ifdef CONFIG_SMP
>         cpus_setall(desc->affinity);
> #endif
> 
> it seems should use irq_default_affinity instead of CPU_MASK_ALL directly there.

That sounds more like it.

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-11-09  2:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-04 16:31 [PATCH 1/1] irq: Maintain user set affinity Kumar Gala
2008-11-06 21:35 ` Andrew Morton
2008-11-06 23:23   ` Kumar Gala
2008-11-08 22:13   ` Yinghai Lu
2008-11-08 22:12 ` Yinghai Lu
2008-11-08 22:17   ` Benjamin Herrenschmidt
2008-11-09  0:01     ` Yinghai Lu
2008-11-09  2:17       ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).