Skip to contentSkip to conversation

irq handling code consolidation (common part)

RFC In review Andrey Panin 3 participants 7 messages Dec 24, 2002 – Jan 4, 2003

lore
Share & tools
lore
#0 lore raw

RFC 1/1 irq handling code consolidation (common part)

Hi all,

this patch moves some common parts of irq handling code to one place. Arch specific patches will follow. Patch for i386 is tested and performed well, but other arch specific patched are not. Please take a look.

Please CC me answering this letter, I'm not subscribed to lkml currently.

Best regards.

Signature · 2 lines
Andrey Panin		| Embedded systems software developer
[email protected]	| PGP key: wwwkeys.pgp.net
kernel/Makefile +1 -0
diff --minimal -urN -X /usr/share/dontdiff linux-2.5.52.vanilla/kernel/Makefile linux-2.5.52/kernel/Makefile
@@ -22,6 +22,7 @@
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o
 obj-$(CONFIG_COMPAT) += compat.o
+obj-$(CONFIG_GENERIC_IRQ) += irq.o
 
 ifneq ($(CONFIG_IA64),y)
 # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is
kernel/irq.c +364 -0

364 lines not on this page · raw message

#1 lore raw
Signature · 3 lines
-- 
Come now, if we were really planning to harm you, would we be waiting here, 
 beside the path, in the very darkest part of the forest?
#2 lore raw
In reply to Andrey Panin
Quoted 3 lines · Andrey Panin

this patch moves some common parts of irq handling code to one place. Arch specific patches will follow. Patch for i386 is tested and performed well, but other arch specific patched are not. Please take a look.

Hmm, well it looks like it will work perfectly with the v850 (which makes sense as it's mostly a copy of the i386 code).

What about request_irq/setup_irq? The majority of architectures use exactly the same code as i386 for these; a few do not, so perhaps this is a case where a HAVE_ARCH_... define could be used.

[setup_irq even has this comment:

/* this was setup_x86_irq but it seems pretty generic */ int setup_irq(unsigned int irq, struct irqaction * new)
]

-Miles

Signature · 2 lines
Come now, if we were really planning to harm you, would we be waiting here, 
 beside the path, in the very darkest part of the forest?
#3 lore raw
Quoted 7 lines · Miles Bader
Quoted 3 lines · Andrey Panin

this patch moves some common parts of irq handling code to one place. Arch specific patches will follow. Patch for i386 is tested and performed well, but other arch specific patched are not. Please take a look.

Hmm, well it looks like it will work perfectly with the v850 (which makes sense as it's mostly a copy of the i386 code).

I have a patch for v850 already, I'll send it soon.

Quoted 9 lines

What about request_irq/setup_irq? The majority of architectures use exactly the same code as i386 for these; a few do not, so perhaps this is a case where a HAVE_ARCH_... define could be used.

[setup_irq even has this comment:

/* this was setup_x86_irq but it seems pretty generic */ int setup_irq(unsigned int irq, struct irqaction * new)
]

It will be the next part of work. I'm changing my job now and lack of time limits my perfomance :(

Quoted 4 lines

-Miles

Signature · 2 lines
Come now, if we were really planning to harm you, would we be waiting here, 
 beside the path, in the very darkest part of the forest?
Signature · 2 lines
Andrey Panin		| Embedded systems software developer
[email protected]	| PGP key: wwwkeys.pgp.net
#4 lore raw
In reply to Andrey Panin
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Andrey Panin

Andrey> Hi all, this patch moves some common parts of irq handling Andrey> code to one place. Arch specific patches will follow. Patch Andrey> for i386 is tested and performed well, but other arch Andrey> specific patched are not. Please take a look.

Andrey> Please CC me answering this letter, I'm not subscribed to Andrey> lkml currently.

+/*
+ * Controller mappings for all interrupt sources:
+ */
+irq_desc_t irq_desc[NR_IRQS] __cacheline_aligned = {
+ [0 ... NR_IRQS - 1] = {
+ .handler = &no_irq_type,
+ .lock = SPIN_LOCK_UNLOCKED,
+ }
+};

This isn't good. For example, NUMA platforms with per-CPU irqs want to allocate the irq descriptors in local memory. On ia64, we introduced a minimal irq-descriptor API for this purpose:

	/* Return a pointer to the irq descriptor for IRQ.  */
	static inline struct irq_desc * irq_desc (int irq);

	/* Extract the IA-64 vector that corresponds to IRQ.  */
	static inline ia64_vector irq_to_vector (int irq);

	/*
	 * Convert the local IA-64 vector to the corresponding irq number.
	 * This translation is done in the context of the interrupt domain
	 * that the currently executing CPU belongs to.
	 */
	static inline unsigned int local_vector_to_irq (ia64_vector vec);

I think the platform-independent part of the code really would only need the first routine irq_desc(). The other two are ia64-specific.

BTW: if you haven't done so already, I'd suggest to take a look at arch/ia64/kernel/irq.c. I tried to keep this code as close as possible to the x86 version. There shouldn't be anything in there that isn't wanted for a good reason.

Thanks,

	--david
#5 lore raw
Quoted 20 lines · David Mosberger
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Andrey Panin

Andrey> Hi all, this patch moves some common parts of irq handling Andrey> code to one place. Arch specific patches will follow. Patch Andrey> for i386 is tested and performed well, but other arch Andrey> specific patched are not. Please take a look.

Andrey> Please CC me answering this letter, I'm not subscribed to Andrey> lkml currently.

kernel/irq.c +9 -0
+/*
+ * Controller mappings for all interrupt sources:
+ */
+irq_desc_t irq_desc[NR_IRQS] __cacheline_aligned = {
+	[0 ... NR_IRQS - 1] = {
+		.handler =	&no_irq_type,
+		.lock =		SPIN_LOCK_UNLOCKED,
+	}
+};
Quoted 3 lines

This isn't good. For example, NUMA platforms with per-CPU irqs want to allocate the irq descriptors in local memory. On ia64, we introduced a minimal irq-descriptor API for this purpose:

I noticed this already, in the new patch which I want to post today irq_desc declaration is guarded by #ifndef HAVE_ARCH_IRQ_DESC.

Quoted 14 lines
	/* Return a pointer to the irq descriptor for IRQ.  */
	static inline struct irq_desc * irq_desc (int irq);

	/* Extract the IA-64 vector that corresponds to IRQ.  */
	static inline ia64_vector irq_to_vector (int irq);

	/*
	 * Convert the local IA-64 vector to the corresponding irq number.
	 * This translation is done in the context of the interrupt domain
	 * that the currently executing CPU belongs to.
	 */
	static inline unsigned int local_vector_to_irq (ia64_vector vec);
Quoted 7 lines

I think the platform-independent part of the code really would only need the first routine irq_desc(). The other two are ia64-specific.

BTW: if you haven't done so already, I'd suggest to take a look at arch/ia64/kernel/irq.c. I tried to keep this code as close as possible to the x86 version. There shouldn't be anything in there that isn't wanted for a good reason.

Already done.

BTW: what is the state of ia64 port in stock 2.5 ? It looks horribly borken :(

Best regards.

Signature · 2 lines
Andrey Panin		| Embedded systems software developer
[email protected]	| PGP key: wwwkeys.pgp.net
#6 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Andrey Panin

Andrey> BTW: what is the state of ia64 port in stock 2.5 ? It looks Andrey> horribly borken :(

You need the matching patch from ftp://ftp.kernel.org/pub/linux/kernel/ports/ia64/v2.5/

	--david