Skip to contentSkip to conversation

bug in irq_affinity_write_proc

PATCH In review Kenji Kaneshige 2 participants 4 messages Jul 21, 2004 – Jul 26, 2004

lore
Share & tools
lore

Kenji Kaneshige

author
#0 lore raw

PATCH 1/1 bug in irq_affinity_write_proc

Hi,

Writing a value with 'R' or 'r' character to /proc/irq/<IRQ#>/smp_affinity causes a kernel Oops. The following patch fixes this issue.

Signed-off-by Kenji Kaneshige <[email protected]>

---

 linux-2.6.8-rc2-kanesige/arch/ia64/kernel/irq.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
arch/ia64/kernel/irq.c renamed from arch/ia64/kernel/irq.c~fix_irq_affinity_write_proc +6 -2
diff -puN arch/ia64/kernel/irq.c~fix_irq_affinity_write_proc arch/ia64/kernel/irq.c
@@ -88,6 +88,7 @@ irq_desc_t _irq_desc[NR_IRQS] __cachelin
  * This is updated when the user sets irq affinity via /proc
  */
 cpumask_t    __cacheline_aligned pending_irq_cpumask[NR_IRQS];
+static int pending_irq_redir[NR_IRQS];
 
 #ifdef CONFIG_IA64_GENERIC
 irq_desc_t * __ia64_irq_desc (unsigned int irq)

@@ -973,6 +974,7 @@ static int irq_affinity_write_proc (stru
 	int prelen;
 	irq_desc_t *desc = irq_descp(irq);
 	unsigned long flags;
+	int redir = 0;
 
 	if (!desc->handler->set_affinity)
 		return -EIO;

@@ -995,7 +997,7 @@ static int irq_affinity_write_proc (stru
 	prelen = 0;
 	if (tolower(*rbuf) = 'r') {
 		prelen = strspn(rbuf, "Rr ");
-		irq |= IA64_IRQ_REDIRECTED;
+		redir++;
 	}
 
 	err = cpumask_parse(buffer+prelen, count-prelen, new_value);

@@ -1012,6 +1014,7 @@ static int irq_affinity_write_proc (stru
 		return -EINVAL;
 
 	spin_lock_irqsave(&desc->lock, flags);
+	pending_irq_redir[irq] = redir;
 	pending_irq_cpumask[irq] = new_value;
 	spin_unlock_irqrestore(&desc->lock, flags);
 

@@ -1023,11 +1026,12 @@ void move_irq(int irq)
 	/* note - we hold desc->lock */
 	cpumask_t tmp;
 	irq_desc_t *desc = irq_descp(irq);
+	int irq_with_redir = pending_irq_redir[irq] ? (irq | IA64_IRQ_REDIRECTED) : irq;
 
 	if (!cpus_empty(pending_irq_cpumask[irq])) {
 		cpus_and(tmp, pending_irq_cpumask[irq], cpu_online_map);
 		if (unlikely(!cpus_empty(tmp))) {
-			desc->handler->set_affinity(irq, pending_irq_cpumask[irq]);
+			desc->handler->set_affinity(irq_with_redir, pending_irq_cpumask[irq]);
 		}
 		cpus_clear(pending_irq_cpumask[irq]);
 	}

_

#1 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Kenji Kaneshige

Kenji> Hi, Writing a value with 'R' or 'r' character to Kenji> /proc/irq/<IRQ#>/smp_affinity causes a kernel Oops. The Kenji> following patch fixes this issue.

Kenji> Signed-off-by: Kenji Kaneshige
Kenji> <[email protected]>

How about using a bitset for pending_irq_redir? Or at least a byte-array instead of a word-array.

	--david
author
#2 lore raw
In reply to Kenji Kaneshige
Quoted 2 lines

How about using a bitset for pending_irq_redir? Or at least a byte-array instead of a word-array.

I modified my patch to use bitset for pending_irq_redir. Please take a look.

Thanks,
Kenji Kaneshige

Writing 'R' or 'r' character to /proc/irq/<IRQ#>/smp_affinity cause a kernel Oops. The following patch fixes this issue.

Signed-off-by Kenji Kaneshige <[email protected]>

---

 linux-2.6.8-rc2-kanesige/arch/ia64/kernel/irq.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)
arch/ia64/kernel/irq.c renamed from arch/ia64/kernel/irq.c~fix_irq_affinity_write_proc +9 -2
diff -puN arch/ia64/kernel/irq.c~fix_irq_affinity_write_proc arch/ia64/kernel/irq.c
@@ -88,6 +88,7 @@ irq_desc_t _irq_desc[NR_IRQS] __cachelin
  * This is updated when the user sets irq affinity via /proc
  */
 cpumask_t    __cacheline_aligned pending_irq_cpumask[NR_IRQS];
+static unsigned long pending_irq_redir[BITS_TO_LONGS(NR_IRQS)];
 
 #ifdef CONFIG_IA64_GENERIC
 irq_desc_t * __ia64_irq_desc (unsigned int irq)

@@ -973,6 +974,7 @@ static int irq_affinity_write_proc (stru
 	int prelen;
 	irq_desc_t *desc = irq_descp(irq);
 	unsigned long flags;
+	int redir = 0;
 
 	if (!desc->handler->set_affinity)
 		return -EIO;

@@ -995,7 +997,7 @@ static int irq_affinity_write_proc (stru
 	prelen = 0;
 	if (tolower(*rbuf) = 'r') {
 		prelen = strspn(rbuf, "Rr ");
-		irq |= IA64_IRQ_REDIRECTED;
+		redir++;
 	}
 
 	err = cpumask_parse(buffer+prelen, count-prelen, new_value);

@@ -1013,6 +1015,10 @@ static int irq_affinity_write_proc (stru
 
 	spin_lock_irqsave(&desc->lock, flags);
 	pending_irq_cpumask[irq] = new_value;
+	if (redir)
+		set_bit(irq, pending_irq_redir);
+	else
+		clear_bit(irq, pending_irq_redir);
 	spin_unlock_irqrestore(&desc->lock, flags);
 
 	return full_count;

@@ -1023,11 +1029,12 @@ void move_irq(int irq)
 	/* note - we hold desc->lock */
 	cpumask_t tmp;
 	irq_desc_t *desc = irq_descp(irq);
+	int irq_with_redir = test_bit(irq, pending_irq_redir) ? (irq | IA64_IRQ_REDIRECTED) : irq;
 
 	if (!cpus_empty(pending_irq_cpumask[irq])) {
 		cpus_and(tmp, pending_irq_cpumask[irq], cpu_online_map);
 		if (unlikely(!cpus_empty(tmp))) {
-			desc->handler->set_affinity(irq, pending_irq_cpumask[irq]);
+			desc->handler->set_affinity(irq_with_redir, pending_irq_cpumask[irq]);
 		}
 		cpus_clear(pending_irq_cpumask[irq]);
 	}

_

#3 lore raw
In reply to Kenji Kaneshige
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line · Kenji Kaneshige

How about using a bitset for pending_irq_redir? Or at least a byte-array instead of a word-array.

Kenji> I modified my patch to use bitset for pending_irq_redir. Kenji> Please take a look.

OK, I checked that in now, with an additional small change which I think improves readability a bit (see below).

Thanks,

	--david

--

arch/ia64/kernel/irq.c +3 -2
diff -Nru a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
@@ -1029,12 +1029,13 @@
 	/* note - we hold desc->lock */
 	cpumask_t tmp;
 	irq_desc_t *desc = irq_descp(irq);
-	int irq_with_redir = test_bit(irq, pending_irq_redir) ? (irq | IA64_IRQ_REDIRECTED) : irq;
+	int redir = test_bit(irq, pending_irq_redir);
 
 	if (!cpus_empty(pending_irq_cpumask[irq])) {
 		cpus_and(tmp, pending_irq_cpumask[irq], cpu_online_map);
 		if (unlikely(!cpus_empty(tmp))) {
-			desc->handler->set_affinity(irq_with_redir, pending_irq_cpumask[irq]);
+			desc->handler->set_affinity(irq | (redir ? IA64_IRQ_REDIRECTED : 0),
+						    pending_irq_cpumask[irq]);
 		}
 		cpus_clear(pending_irq_cpumask[irq]);
 	}