Skip to contentSkip to conversation

fixing the irqwcpustat mess

RFC PATCH In review Christoph Hellwig 4 participants 6 messages May 5, 2003 – May 7, 2003

lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

RFC PATCH 1/1 fixing the irqwcpustat mess

currently only x86_64 and ia64 don't use the generic irq_cpustat code and both have to workaround it's brokenness for the non-default case.

x86_64 defines an empty irq_cpustat_t even if it doesn't need one and ia64 adds CONFIG_IA64 ifdefs around all users. What about this patch instead to make __ARCH_IRQ_STAT useable?

include/asm-ia64/hardirq.h +3 -0
@@ -16,6 +16,9 @@
 /*
  * No irq_cpustat_t for IA-64.  The data is held in the per-CPU data structure.
  */
+
+#define __ARCH_IRQ_STAT	1
+
 #define softirq_pending(cpu)		(cpu_data(cpu)->softirq_pending)
 #define syscall_count(cpu)		/* unused on IA-64 */
 #define ksoftirqd_task(cpu)		(cpu_data(cpu)->ksoftirqd)
include/asm-x86_64/hardirq.h +0 -4
@@ -12,10 +12,6 @@
    special access macros. This would generate better code. */ 
 #define __IRQ_STAT(cpu,member) (read_pda(me)->member)
 
-typedef struct {
-	/* Empty. All the fields have moved to the PDA. */
-} irq_cpustat_t; 
-
 #include <linux/irq_cpustat.h>	/* Standard mappings for irq_cpustat_t above */
 
 /*
include/linux/irq_cpustat.h +6 -4
@@ -17,9 +17,8 @@
  * definitions instead of differing sets for each arch.
  */
 
-extern irq_cpustat_t irq_stat[];			/* defined in asm/hardirq.h */
-
-#ifndef __ARCH_IRQ_STAT /* Some architectures can do this more efficiently */ 
+#ifndef __ARCH_IRQ_STAT
+extern irq_cpustat_t irq_stat[];		/* defined in asm/hardirq.h */
 #ifdef CONFIG_SMP
 #define __IRQ_STAT(cpu, member)	(irq_stat[cpu].member)
 #else

@@ -31,8 +30,11 @@
 #define softirq_pending(cpu)	__IRQ_STAT((cpu), __softirq_pending)
 #define local_softirq_pending()	softirq_pending(smp_processor_id())
 #define syscall_count(cpu)	__IRQ_STAT((cpu), __syscall_count)
+#define local_syscall_count()	syscall_count(smp_processor_id())
 #define ksoftirqd_task(cpu)	__IRQ_STAT((cpu), __ksoftirqd_task)
+#define local_ksoftirqd_task()	ksoftirqd_task(smp_processor_id())
+
   /* arch dependent irq_stat fields */
-#define nmi_count(cpu)		__IRQ_STAT((cpu), __nmi_count)		/* i386, ia64 */
+#define nmi_count(cpu)		__IRQ_STAT((cpu), __nmi_count)	/* i386 */
 
 #endif	/* __irq_cpustat_h */
kernel/ksyms.c +0 -1
@@ -400,7 +400,6 @@
 EXPORT_SYMBOL(del_timer);
 EXPORT_SYMBOL(request_irq);
 EXPORT_SYMBOL(free_irq);
-EXPORT_SYMBOL(irq_stat);
 
 /* waitqueue handling */
 EXPORT_SYMBOL(add_wait_queue);
kernel/softirq.c +4 -1
@@ -33,7 +33,10 @@
    - Tasklets: serialized wrt itself.
  */
 
+#ifndef __ARCH_IRQ_STAT
 irq_cpustat_t irq_stat[NR_CPUS] ____cacheline_aligned;
+EXPORT_SYMBOL(irq_stat);
+#endif
 
 static struct softirq_action softirq_vec[32] __cacheline_aligned_in_smp;
 

@@ -321,7 +324,7 @@
 	__set_current_state(TASK_INTERRUPTIBLE);
 	mb();
 
-	ksoftirqd_task(cpu) = current;
+	local_ksoftirqd_task() = current;
 
 	for (;;) {
 		if (!local_softirq_pending())
#1 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Christoph Hellwig

Christoph> What about this patch instead to make __ARCH_IRQ_STAT Christoph> useable?

Looks great to me.

Thanks,

	--david
#2 lore raw
In reply to Christoph Hellwig

kernel/softirq.c line 39

Quoted 5 lines · Christoph Hellwig
kernel/softirq.c +3 -0
+#ifndef __ARCH_IRQ_STAT
 irq_cpustat_t irq_stat[NR_CPUS] ____cacheline_aligned;
+EXPORT_SYMBOL(irq_stat);
+#endif

Shouldn't this be per_cpu_data ?

-Andi

#3 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Andi Kleen
Quoted 2 lines · Christoph Hellwig
Quoted 2 lines

+#ifndef __ARCH_IRQ_STAT irq_cpustat_t irq_stat[NR_CPUS]
____cacheline_aligned; +EXPORT_SYMBOL(irq_stat); +#endif

Andi> Shouldn't this be per_cpu_data ?

That's what we do on ia64. Though we actually end up putting it in the cpuinfo structure, for historical reasons but also to ensure that it's close to some other stuff that's used on the irq path.

	--david
#4 lore raw
Quoted 11 lines · David Mosberger-Tang
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Andi Kleen
Quoted 2 lines · Christoph Hellwig
Quoted 2 lines

+#ifndef __ARCH_IRQ_STAT irq_cpustat_t irq_stat[NR_CPUS]
____cacheline_aligned; +EXPORT_SYMBOL(irq_stat); +#endif

Andi> Shouldn't this be per_cpu_data ?

That's what we do on ia64. Though we actually end up putting it in the cpuinfo structure, for historical reasons but also to ensure that it's close to some other stuff that's used on the irq path.

x86-64 does the same (putting it into the PDA). Was just commenting generically for the other ports.

-Andi

author
#5 lore raw
In reply to Andi Kleen
Quoted 9 lines · Andi Kleen

kernel/softirq.c line 39

Quoted 5 lines · Christoph Hellwig
kernel/softirq.c +3 -0
+#ifndef __ARCH_IRQ_STAT
 irq_cpustat_t irq_stat[NR_CPUS] ____cacheline_aligned;
+EXPORT_SYMBOL(irq_stat);
+#endif

Shouldn't this be per_cpu_data ?

Yes, it should. I have a patch for this but it' a separate issue.