* [PATCH 1/2] sparc64: Implement local_irq_save_nmi().
@ 2010-04-06 23:39 David Miller
2010-04-06 23:39 ` David Miller
2010-04-07 6:52 ` Peter Zijlstra
0 siblings, 2 replies; 7+ messages in thread
From: David Miller @ 2010-04-06 23:39 UTC (permalink / raw)
To: a.p.zijlstra
Cc: mingo, acme, paulus, efault, fweisbec, tglx, linux-kernel,
sparclinux, linux-arch
It disables up to PIL_NMI instead of just PIL_NORMAL_MAX.
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/arch/sparc/include/asm/irqflags_64.h b/arch/sparc/include/asm/irqflags_64.h
index 8b49bf9..fa1e00e 100644
--- a/arch/sparc/include/asm/irqflags_64.h
+++ b/arch/sparc/include/asm/irqflags_64.h
@@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
);
}
+static inline void raw_local_irq_disable_nmi(void)
+{
+ __asm__ __volatile__(
+ "wrpr %0, %%pil"
+ : /* no outputs */
+ : "i" (PIL_NMI)
+ : "memory"
+ );
+}
+
static inline void raw_local_irq_enable(void)
{
__asm__ __volatile__(
@@ -83,9 +93,28 @@ static inline unsigned long __raw_local_irq_save(void)
return flags;
}
+static inline unsigned long __raw_local_irq_save_nmi(void)
+{
+ unsigned long flags = __raw_local_save_flags();
+
+ raw_local_irq_disable_nmi();
+
+ return flags;
+}
+
#define raw_local_irq_save(flags) \
do { (flags) = __raw_local_irq_save(); } while (0)
+#define raw_local_irq_save_nmi(flags) \
+ do { (flags) = __raw_local_irq_save_nmi(); } while (0)
+
+#define local_irq_save_nmi(flags) \
+ do { \
+ typecheck(unsigned long, flags); \
+ raw_local_irq_save_nmi(flags); \
+ trace_hardirqs_off(); \
+ } while (0)
+
#endif /* (__ASSEMBLY__) */
#endif /* !(_ASM_IRQFLAGS_H) */
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 1/2] sparc64: Implement local_irq_save_nmi().
2010-04-06 23:39 [PATCH 1/2] sparc64: Implement local_irq_save_nmi() David Miller
@ 2010-04-06 23:39 ` David Miller
2010-04-07 6:52 ` Peter Zijlstra
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2010-04-06 23:39 UTC (permalink / raw)
To: a.p.zijlstra
Cc: mingo, acme, paulus, efault, fweisbec, tglx, linux-kernel,
sparclinux, linux-arch
It disables up to PIL_NMI instead of just PIL_NORMAL_MAX.
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/arch/sparc/include/asm/irqflags_64.h b/arch/sparc/include/asm/irqflags_64.h
index 8b49bf9..fa1e00e 100644
--- a/arch/sparc/include/asm/irqflags_64.h
+++ b/arch/sparc/include/asm/irqflags_64.h
@@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
);
}
+static inline void raw_local_irq_disable_nmi(void)
+{
+ __asm__ __volatile__(
+ "wrpr %0, %%pil"
+ : /* no outputs */
+ : "i" (PIL_NMI)
+ : "memory"
+ );
+}
+
static inline void raw_local_irq_enable(void)
{
__asm__ __volatile__(
@@ -83,9 +93,28 @@ static inline unsigned long __raw_local_irq_save(void)
return flags;
}
+static inline unsigned long __raw_local_irq_save_nmi(void)
+{
+ unsigned long flags = __raw_local_save_flags();
+
+ raw_local_irq_disable_nmi();
+
+ return flags;
+}
+
#define raw_local_irq_save(flags) \
do { (flags) = __raw_local_irq_save(); } while (0)
+#define raw_local_irq_save_nmi(flags) \
+ do { (flags) = __raw_local_irq_save_nmi(); } while (0)
+
+#define local_irq_save_nmi(flags) \
+ do { \
+ typecheck(unsigned long, flags); \
+ raw_local_irq_save_nmi(flags); \
+ trace_hardirqs_off(); \
+ } while (0)
+
#endif /* (__ASSEMBLY__) */
#endif /* !(_ASM_IRQFLAGS_H) */
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] sparc64: Implement local_irq_save_nmi().
2010-04-06 23:39 [PATCH 1/2] sparc64: Implement local_irq_save_nmi() David Miller
2010-04-06 23:39 ` David Miller
@ 2010-04-07 6:52 ` Peter Zijlstra
2010-04-07 6:52 ` Peter Zijlstra
2010-04-07 7:06 ` David Miller
1 sibling, 2 replies; 7+ messages in thread
From: Peter Zijlstra @ 2010-04-07 6:52 UTC (permalink / raw)
To: David Miller
Cc: mingo, acme, paulus, efault, fweisbec, tglx, linux-kernel,
sparclinux, linux-arch
On Tue, 2010-04-06 at 16:39 -0700, David Miller wrote:
> It disables up to PIL_NMI instead of just PIL_NORMAL_MAX.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/arch/sparc/include/asm/irqflags_64.h b/arch/sparc/include/asm/irqflags_64.h
> index 8b49bf9..fa1e00e 100644
> --- a/arch/sparc/include/asm/irqflags_64.h
> +++ b/arch/sparc/include/asm/irqflags_64.h
> @@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
> );
> }
>
> +static inline void raw_local_irq_disable_nmi(void)
> +{
> + __asm__ __volatile__(
> + "wrpr %0, %%pil"
> + : /* no outputs */
> + : "i" (PIL_NMI)
> + : "memory"
> + );
> +}
> +
Isn't this wrong when used from !NMI context?
Should this thing do something like:
if (rdpr() < PIL_NORMAL_MAX)
wrpr(PIL_NORMAL_MAX);
so that it only disables IRQs, but doesn't enable NMIs.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] sparc64: Implement local_irq_save_nmi().
2010-04-07 6:52 ` Peter Zijlstra
@ 2010-04-07 6:52 ` Peter Zijlstra
2010-04-07 7:06 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2010-04-07 6:52 UTC (permalink / raw)
To: David Miller
Cc: mingo, acme, paulus, efault, fweisbec, tglx, linux-kernel,
sparclinux, linux-arch
On Tue, 2010-04-06 at 16:39 -0700, David Miller wrote:
> It disables up to PIL_NMI instead of just PIL_NORMAL_MAX.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/arch/sparc/include/asm/irqflags_64.h b/arch/sparc/include/asm/irqflags_64.h
> index 8b49bf9..fa1e00e 100644
> --- a/arch/sparc/include/asm/irqflags_64.h
> +++ b/arch/sparc/include/asm/irqflags_64.h
> @@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
> );
> }
>
> +static inline void raw_local_irq_disable_nmi(void)
> +{
> + __asm__ __volatile__(
> + "wrpr %0, %%pil"
> + : /* no outputs */
> + : "i" (PIL_NMI)
> + : "memory"
> + );
> +}
> +
Isn't this wrong when used from !NMI context?
Should this thing do something like:
if (rdpr() < PIL_NORMAL_MAX)
wrpr(PIL_NORMAL_MAX);
so that it only disables IRQs, but doesn't enable NMIs.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] sparc64: Implement local_irq_save_nmi().
2010-04-07 6:52 ` Peter Zijlstra
2010-04-07 6:52 ` Peter Zijlstra
@ 2010-04-07 7:06 ` David Miller
2010-04-07 7:24 ` Peter Zijlstra
1 sibling, 1 reply; 7+ messages in thread
From: David Miller @ 2010-04-07 7:06 UTC (permalink / raw)
To: peterz
Cc: mingo, acme, paulus, efault, fweisbec, tglx, linux-kernel,
sparclinux, linux-arch
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed, 07 Apr 2010 08:52:26 +0200
> On Tue, 2010-04-06 at 16:39 -0700, David Miller wrote:
>> @@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
>> );
>> }
>>
>> +static inline void raw_local_irq_disable_nmi(void)
>> +{
>> + __asm__ __volatile__(
>> + "wrpr %0, %%pil"
>> + : /* no outputs */
>> + : "i" (PIL_NMI)
>> + : "memory"
>> + );
>> +}
>> +
>
> Isn't this wrong when used from !NMI context?
>
> Should this thing do something like:
>
> if (rdpr() < PIL_NORMAL_MAX)
> wrpr(PIL_NORMAL_MAX);
>
> so that it only disables IRQs, but doesn't enable NMIs.
It's immaterial, local_irq_restore() will do the right thing,
and it's ok to disable NMIs in these few cases I think.
I desperately want to avoid that "test and maybe change the
value %pil value we write" business, and honestly that's
the whole point of this exercise.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] sparc64: Implement local_irq_save_nmi().
2010-04-07 7:06 ` David Miller
@ 2010-04-07 7:24 ` Peter Zijlstra
2010-04-07 7:24 ` Peter Zijlstra
0 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2010-04-07 7:24 UTC (permalink / raw)
To: David Miller
Cc: mingo, acme, paulus, efault, fweisbec, tglx, linux-kernel,
sparclinux, linux-arch
On Wed, 2010-04-07 at 00:06 -0700, David Miller wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed, 07 Apr 2010 08:52:26 +0200
>
> > On Tue, 2010-04-06 at 16:39 -0700, David Miller wrote:
> >> @@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
> >> );
> >> }
> >>
> >> +static inline void raw_local_irq_disable_nmi(void)
> >> +{
> >> + __asm__ __volatile__(
> >> + "wrpr %0, %%pil"
> >> + : /* no outputs */
> >> + : "i" (PIL_NMI)
> >> + : "memory"
> >> + );
> >> +}
> >> +
> >
> > Isn't this wrong when used from !NMI context?
> >
> > Should this thing do something like:
> >
> > if (rdpr() < PIL_NORMAL_MAX)
> > wrpr(PIL_NORMAL_MAX);
> >
> > so that it only disables IRQs, but doesn't enable NMIs.
>
> It's immaterial, local_irq_restore() will do the right thing,
> and it's ok to disable NMIs in these few cases I think.
>
> I desperately want to avoid that "test and maybe change the
> value %pil value we write" business, and honestly that's
> the whole point of this exercise.
Sure, its your architecture.. but could you explain why you're trying to
avoid that compare so desperately, the local_irq_save_nmi() calls are
few so surely they could carry that overhead.
Also, doesn't __raw_local_irq_save_flags() already do the read? So its
really just the compare that's gone missing.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] sparc64: Implement local_irq_save_nmi().
2010-04-07 7:24 ` Peter Zijlstra
@ 2010-04-07 7:24 ` Peter Zijlstra
0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2010-04-07 7:24 UTC (permalink / raw)
To: David Miller
Cc: mingo, acme, paulus, efault, fweisbec, tglx, linux-kernel,
sparclinux, linux-arch
On Wed, 2010-04-07 at 00:06 -0700, David Miller wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed, 07 Apr 2010 08:52:26 +0200
>
> > On Tue, 2010-04-06 at 16:39 -0700, David Miller wrote:
> >> @@ -49,6 +49,16 @@ static inline void raw_local_irq_disable(void)
> >> );
> >> }
> >>
> >> +static inline void raw_local_irq_disable_nmi(void)
> >> +{
> >> + __asm__ __volatile__(
> >> + "wrpr %0, %%pil"
> >> + : /* no outputs */
> >> + : "i" (PIL_NMI)
> >> + : "memory"
> >> + );
> >> +}
> >> +
> >
> > Isn't this wrong when used from !NMI context?
> >
> > Should this thing do something like:
> >
> > if (rdpr() < PIL_NORMAL_MAX)
> > wrpr(PIL_NORMAL_MAX);
> >
> > so that it only disables IRQs, but doesn't enable NMIs.
>
> It's immaterial, local_irq_restore() will do the right thing,
> and it's ok to disable NMIs in these few cases I think.
>
> I desperately want to avoid that "test and maybe change the
> value %pil value we write" business, and honestly that's
> the whole point of this exercise.
Sure, its your architecture.. but could you explain why you're trying to
avoid that compare so desperately, the local_irq_save_nmi() calls are
few so surely they could carry that overhead.
Also, doesn't __raw_local_irq_save_flags() already do the read? So its
really just the compare that's gone missing.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-04-07 7:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-06 23:39 [PATCH 1/2] sparc64: Implement local_irq_save_nmi() David Miller
2010-04-06 23:39 ` David Miller
2010-04-07 6:52 ` Peter Zijlstra
2010-04-07 6:52 ` Peter Zijlstra
2010-04-07 7:06 ` David Miller
2010-04-07 7:24 ` Peter Zijlstra
2010-04-07 7:24 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox