The mysterious case of struct irqaction's mask field.

The mysterious case of struct irqaction's mask field.

[Rusty Russell]

Hi all,

   As part of the cpumask conversion, I came across struct irqaction:

	struct irqaction {
		irq_handler_t handler;
		unsigned long flags;
		cpumask_t mask;
		...
	};

Most people have been setting 'mask' to CPU_MASK_NONE, and I wondered if that
really meant that they never want this action performed on any CPU.

But I couldn't find anyone who actually *reads* the 'mask' field.  Tracing
back, it was converted from an unsigned long to a cpumask_t by wli around
2.6.7 ("as it was intended to be").  But that conversion didn't reveal anyone
actually using the field either.

At one point, sparc64 seems to have overloaded it for some kind of irq bucket
scheme.

Finally, I tracked it back to the creation of (then per-arch) struct irqaction
in 1.1.82, and this hunk from linux/arch/i386/kernel/irq.c:

@@ -12,14 +12,7 @@

 /*
  * IRQ's are in fact implemented a bit like signal handlers for the kernel.
- * The same sigaction struct is used, and with similar semantics (ie there
- * is a SA_INTERRUPT flag etc). Naturally it's not a 1:1 relation, but there
- * are similarities.
- *
- * sa_handler(int irq_NR) is the default function called (0 if no).
- * sa_mask is horribly ugly (I won't even mention it)
- * sa_flags contains various info: SA_INTERRUPT etc
- * sa_restorer is the unused
+ * Naturally it's not a 1:1 relation, but there are similarities.
  */

 #include <linux/ptrace.h>

So, it was never a cpumask at all; just a remanent of the use of sigaction for
interrupt handlers.  We've been happily setting it throughout the kernel since
1995.

On the assumption that it has failed to coerce the spirits of our ancestors to
land among us, I'll create a patch to remove it.

Cheers!
Rusty.

Re: The mysterious case of struct irqaction's mask field.

[David Miller]

From: Rusty Russell <rusty@rustcorp.com.au>
Date: Tue, 10 Feb 2009 12:23:46 +1030

> On the assumption that it has failed to coerce the spirits of our
> ancestors to land among us, I'll create a patch to remove it.

Please do :)

Re: The mysterious case of struct irqaction's mask field.

[Ingo Molnar]

* Rusty Russell <rusty@rustcorp.com.au> wrote:

> Hi all,
> 
>    As part of the cpumask conversion, I came across struct irqaction:
> 
> 	struct irqaction {
> 		irq_handler_t handler;
> 		unsigned long flags;
> 		cpumask_t mask;
> 		...
> 	};
> 
> Most people have been setting 'mask' to CPU_MASK_NONE, and I wondered if that
> really meant that they never want this action performed on any CPU.
> 
> But I couldn't find anyone who actually *reads* the 'mask' field.  Tracing
> back, it was converted from an unsigned long to a cpumask_t by wli around
> 2.6.7 ("as it was intended to be").  But that conversion didn't reveal anyone
> actually using the field either.
> 
> At one point, sparc64 seems to have overloaded it for some kind of irq bucket
> scheme.
> 
> Finally, I tracked it back to the creation of (then per-arch) struct irqaction
> in 1.1.82, and this hunk from linux/arch/i386/kernel/irq.c:
> 
> @@ -12,14 +12,7 @@
> 
>  /*
>   * IRQ's are in fact implemented a bit like signal handlers for the kernel.
> - * The same sigaction struct is used, and with similar semantics (ie there
> - * is a SA_INTERRUPT flag etc). Naturally it's not a 1:1 relation, but there
> - * are similarities.
> - *
> - * sa_handler(int irq_NR) is the default function called (0 if no).
> - * sa_mask is horribly ugly (I won't even mention it)
> - * sa_flags contains various info: SA_INTERRUPT etc
> - * sa_restorer is the unused
> + * Naturally it's not a 1:1 relation, but there are similarities.
>   */
> 
>  #include <linux/ptrace.h>
> 
> So, it was never a cpumask at all; just a remanent of the use of sigaction for
> interrupt handlers.  We've been happily setting it throughout the kernel since
> 1995.

Hehe, nice one :-)

> On the assumption that it has failed to coerce the spirits of our ancestors to
> land among us, I'll create a patch to remove it.

Please do.

This seems to be a classic symptom of write-mostly kernel source code :-/

	Ingo