I believe do_cpu_irq_mask is called with interrupts disabled (virt_map
sets PSW to KERNEL_PSW).
So, the local_irq_disable(); line can go.
It does look like set_irq_regs has to be atomic on a per cpu basis. So,
if interrupts weren't already
disabled, there would be a problem with current code.
Dave
On 10/10/2013 4:32 AM, Helge Deller wrote:
Hi James,
On 10/10/2013 04:17 AM, James Bottomley wrote:
On Wed, 2013-10-09 at 23:54 +0200, Helge Deller wrote:
Signed-off-by: Helge Deller <deller@xxxxxx>
diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index 2e6443b..c439c05 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -529,8 +529,8 @@ void do_cpu_irq_mask(struct pt_regs *regs)
cpumask_t dest;
#endif
- old_regs = set_irq_regs(regs);
local_irq_disable();
+ old_regs = set_irq_regs(regs);
I don't quite understand why. set_irq_regs is just saving the current
regs pointer.
...and setting a new one...
The design intent is to call it first thing in the
interrupt routine but because of the way we use them, it makes no
difference whether you do it before or after disabling interrupts
because it's stacked. What was the reason for wanting to change it to a
non-standard calling pattern?
Is it really non-standard?
My first intention was to align the set_irq_regs() and entrance and exit
to the irq_enter() and irq_exit() functions.
With my change above it's now:
old_regs = set_irq_regs(regs);
irq_enter();
do_something...();
irq_exit();
set_irq_regs(old_regs);
That's the same syntax as all other arches use.
I think the main question is, if we need local_irq_disable() at all?
At least moving the "old_regs = set_irq_regs(regs);" down after local_irq_disable()
ensures that nobody else modifies the irq_regs pointer before we save it into old_regs.
Helge
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
John David Anglin dave.anglin@xxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
