Skip to contentSkip to conversation

fix irq statistics

PATCH In review Jan Kiszka 2 participants 4 messages Sep 19, 2007 – Sep 23, 2007

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 fix irq statistics

Hi,

IRQ hit counters are broken in latest I-pipe 1.10-x for the wired path. This patch moves the counter maintenance out of __ipipe_set_irq_pending and instead makes it explicit at the required spots.

Note that this patch also unexports __ipipe_set_irq_pending because I found no reason why it should be used outside the i-pipe core. Please correct me if I'm wrong on this.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
---
 arch/i386/kernel/ipipe.c |    1 +
 kernel/ipipe/core.c      |    7 +++----
 2 files changed, 4 insertions(+), 4 deletions(-)
arch/i386/kernel/ipipe.c +1 -0
Index: linux-2.6.22-ipipe/arch/i386/kernel/ipipe.c
===================================================================
@@ -727,6 +727,7 @@ int __ipipe_handle_irq(struct pt_regs re
 			 * _first_ in the domain's status flags before
 			 * the PIC is unlocked.
 			 */
+			ipipe_cpudom_var(next_domain, irqall)[irq]++;
 			__ipipe_set_irq_pending(next_domain, irq);
 
 			if (!m_ack && next_domain->irqs[irq].acknowledge != NULL)
kernel/ipipe/core.c +3 -4
Index: linux-2.6.22-ipipe/kernel/ipipe/core.c
===================================================================
@@ -493,8 +493,6 @@ void fastcall __ipipe_set_irq_pending(st
 		__set_bit(level,&ipipe_cpudom_var(ipd, irqpend_himask));
 	} else
 		__set_bit(rank, &ipipe_cpudom_var(ipd, irqheld_mask)[level]);
-
-	ipipe_cpudom_var(ipd, irqall)[irq]++;
 }
 
 /* Must be called hw IRQs off. */

@@ -873,12 +871,13 @@ int fastcall __ipipe_dispatch_wired(stru
 {
 	struct ipipe_domain *old;
 
+	ipipe_cpudom_var(head_domain, irqall)[irq]++;
+
 	if (test_bit(IPIPE_LOCK_FLAG, &head_domain->irqs[irq].control)) {
 		/* If we can't process this IRQ right now, we must
 		 * mark it as held, so that it will get played during
 		 * normal log sync when the corresponding interrupt
 		 * source is eventually unlocked. */
-		ipipe_cpudom_var(head_domain, irqall)[irq]++;
 		__set_bit(irq & IPIPE_IRQ_IMASK, &ipipe_cpudom_var(head_domain, irqheld_mask)[irq >> IPIPE_IRQ_ISHIFT]);
 		return 0;
 	}

@@ -1199,6 +1198,7 @@ int fastcall __ipipe_schedule_irq(unsign
 		ipd = list_entry(ln, struct ipipe_domain, p_link);
 
 		if (test_bit(IPIPE_HANDLE_FLAG, &ipd->irqs[irq].control)) {
+			ipipe_cpudom_var(ipd, irqall)[irq]++;
 			__ipipe_set_irq_pending(ipd, irq);
 			local_irq_restore_hw(flags);
 			return 1;

@@ -1597,7 +1597,6 @@ EXPORT_SYMBOL(__ipipe_spin_unlock_irq);
 EXPORT_SYMBOL(__ipipe_spin_lock_irqsave);
 EXPORT_SYMBOL(__ipipe_spin_unlock_irqrestore);
 EXPORT_SYMBOL(__ipipe_pipeline);
-EXPORT_SYMBOL(__ipipe_set_irq_pending);
 EXPORT_SYMBOL(__ipipe_lock_irq);
 EXPORT_SYMBOL(__ipipe_unlock_irq);
 EXPORT_SYMBOL(ipipe_register_domain);
#1 lore raw
Quoted 5 lines · Jan Kiszka

Hi,

IRQ hit counters are broken in latest I-pipe 1.10-x for the wired path. This patch moves the counter maintenance out of __ipipe_set_irq_pending and instead makes it explicit at the required spots.

I would rather add the missing accounting code as below. Marking an interrupt as pending using the __ipipe_set_irq_pending() interface should implicitely be paired with proper accounting.

kernel/ipipe/core.c +1 -0
@@ -786,6 +786,7 @@ int fastcall __ipipe_dispatch_wired(struct ipipe_domain *head_domain, unsigned i
 	old = ipipe_current_domain;
 	ipipe_current_domain = head_domain; /* Switch to the head domain. */
 
+	ipipe_cpudom_var(head_domain, irqall)[irq]++;
 	__set_bit(IPIPE_STALL_FLAG, &ipipe_cpudom_var(head_domain, status));
 	head_domain->irqs[irq].handler(irq, head_domain->irqs[irq].cookie); /* Call the ISR. */
 	__ipipe_run_irqtail();
Quoted 5 lines

Note that this patch also unexports __ipipe_set_irq_pending because I found no reason why it should be used outside the i-pipe core. Please correct me if I'm wrong on this.

That's correct. This export became useless as soon as __ipipe_unlock_irq() and friends moved out of line, and started fiddling directly with the interrupt bitmap internally.

Quoted 2 lines

Jan

Signature · 1 line
Philippe.
author
#2 lore raw
Quoted 10 lines · Philippe Gerum
Quoted 5 lines · Jan Kiszka

Hi,

IRQ hit counters are broken in latest I-pipe 1.10-x for the wired path. This patch moves the counter maintenance out of __ipipe_set_irq_pending and instead makes it explicit at the required spots.

I would rather add the missing accounting code as below. Marking an interrupt as pending using the __ipipe_set_irq_pending() interface should implicitely be paired with proper accounting.

My original intention was to keep __ipipe_dispatch_wired compact.

But thinking about this again, I wonder if the whole thing makes sense as it is implemented right now: Given that I-pipe users like Xenomai now use this irqall counter as high-level hit counter, I'm not that sure we should also account for IRQ events on locked lines or in case of stalled receiver domains. Only _delivery_, ie. handler invocation, should be counted IMO (=>__ipipe_run_isr). What do you think?

Jan

#3 lore raw
Quoted 8 lines · Jan Kiszka
Quoted 7 lines · Philippe Gerum
Quoted 5 lines · Jan Kiszka

Hi,

IRQ hit counters are broken in latest I-pipe 1.10-x for the wired path. This patch moves the counter maintenance out of __ipipe_set_irq_pending and instead makes it explicit at the required spots.

Quoted 5 lines
Quoted 3 lines

I would rather add the missing accounting code as below. Marking an interrupt as pending using the __ipipe_set_irq_pending() interface should implicitely be paired with proper accounting.

My original intention was to keep __ipipe_dispatch_wired compact.

Yes, but my intention in moving this code out of line is to gather bitmap management and accounting.

Quoted 8 lines

But thinking about this again, I wonder if the whole thing makes sense as it is implemented right now: Given that I-pipe users like Xenomai now use this irqall counter as high-level hit counter, I'm not that sure we should also account for IRQ events on locked lines or in case of stalled receiver domains. Only _delivery_, ie. handler invocation, should be counted IMO (=>__ipipe_run_isr). What do you think?

Now that the interrupt log is flat, we do not have any pending IRQ counter anymore. So the only way to determine how many interrupts are actually waiting for being processed is to substract the irq_all value from a private dispatch counter maintained by the client code from its own handlers. So regardless of what Xenomai does now, I would rather keep this option open, so that domains may compute this information on their own.

Quoted 2 lines

Jan

Signature · 1 line
Philippe.