All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Dimitri Sivanich <sivanich@sgi.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	john stultz <johnstul@us.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6 v6] SGI RTC: add generic system interrupt
Date: Tue, 3 Mar 2009 16:34:33 +0100	[thread overview]
Message-ID: <20090303153433.GA29454@elte.hu> (raw)
In-Reply-To: <20090303151357.GA20120@sgi.com>


* Dimitri Sivanich <sivanich@sgi.com> wrote:

> This patch allocates a system interrupt vector for various 
> platform specific uses.

this is really ugly.

Also, why are all these symbols exported? There's no need to 
build the UV RTC driver as a module. It's either built-in or not 
built-in - it's small enough.

this stuff:

> +/* Function pointer for generic interrupt vector handling */
> +static void (*generic_interrupt_extension)(void);
> +static char generic_show_string[28];
> +static char generic_show_prefix[6];
> +
> +int is_generic_interrupt_registered()
> +{
> +	if (generic_interrupt_extension)
> +		return 1;
> +	else
> +		return 0;
> +}
> +
> +char *generic_interrupt_string(void)
> +{
> +	return generic_show_string;
> +}
> +
> +char *generic_interrupt_prefix(void)
> +{
> +	return generic_show_prefix;
> +}

is SMP unsafe, etc. etc. - not something we should ever call 
from a module. We just shouldnt do it in this form. What 
necessiates it?

All we need is:

> +	/* generic IPI for platform specific use */
> +	alloc_intr_gate(GENERIC_INTERRUPT_VECTOR, generic_interrupt);

plus one trivial callback function - and then the UV platform 
uses it for its own purpose. It's not like two platforms will be 
running at once so there's no locking needed, etc.

> +#ifdef CONFIG_X86_64
> +extern void generic_interrupt(void);
> +#endif

in any case please make it symmetric across 32-bit and 64-bit - 
even though UV is 64-bit only.

	Ingo

  parent reply	other threads:[~2009-03-03 15:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-03 15:10 [PATCH 0/6 v6] SGI RTC: add clocksource/clockevent driver and generic vector Dimitri Sivanich
2009-03-03 15:13 ` [PATCH 1/6 v6] SGI RTC: add generic system interrupt Dimitri Sivanich
2009-03-03 15:15   ` [PATCH 2/6 v6] SGI RTC: export clocksource_unregister Dimitri Sivanich
2009-03-03 15:16     ` [PATCH 3/6 v6] SGI RTC: export clockevents_register_device Dimitri Sivanich
2009-03-03 15:18       ` [PATCH 4/6 v6] SGI RTC: export schedule_on_each_cpu Dimitri Sivanich
2009-03-03 15:20         ` [PATCH 5/6 v6] SGI RTC: loop through installed UV blades Dimitri Sivanich
2009-03-03 15:22           ` [PATCH 6/6 v6] SGI RTC: add clocksource driver Dimitri Sivanich
2009-03-03 15:35             ` Ingo Molnar
2009-03-03 21:59               ` H. Peter Anvin
2009-03-03 20:01             ` john stultz
2009-03-03 20:25               ` Dimitri Sivanich
2009-03-03 15:34   ` Ingo Molnar [this message]
2009-03-03 17:39     ` [PATCH 1/6 v6] SGI RTC: add generic system interrupt Dimitri Sivanich
2009-03-03 18:52       ` Ingo Molnar

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=20090303153433.GA29454@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sivanich@sgi.com \
    --cc=tglx@linutronix.de \
    /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.