All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	William Lee Irwin III <wli@holomorphy.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: The mysterious case of struct irqaction's mask field.
Date: Tue, 10 Feb 2009 13:23:32 +0100	[thread overview]
Message-ID: <20090210122332.GA28817@elte.hu> (raw)
In-Reply-To: <200902101223.47313.rusty@rustcorp.com.au>


* 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

      parent reply	other threads:[~2009-02-10 12:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-10  1:53 The mysterious case of struct irqaction's mask field Rusty Russell
2009-02-10  2:04 ` David Miller
2009-02-10 12:23 ` Ingo Molnar [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090210122332.GA28817@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=wli@holomorphy.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.