bug in irq_affinity_write_proc
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.
---
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
@@ -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 (struint 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 (strureturn -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]); }
_
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
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.
---
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
@@ -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 (struint 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 (struspin_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]); }
_
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
@@ -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]); }