| Message ID | 20141114154315.GA15771@gondor.apana.org.au (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 14 November 2014 16:43, Herbert Xu <herbert@gondor.apana.org.au> wrote: > While working on the cryptd request reordering problem, I noticed > an anomaly where kernel threads are normally allowed to use simd > per may_use_simd, but as soon as you disable softirqs, they suddenly > lose that ability for no good reason. > > The problem is that in_interrupt does not distinguish between > softirq processing and simply having softirqs disabled. This > patch creates a new helper in_serving_interrupt which makes that > distinction. It then uses this in all current implementations > of may_use_simd. > Isn't that a much more widespread problem if in_interrupt() yields true while no interrupt is being served? > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c > index a9a4229..6cdaa852 100644 > --- a/arch/x86/kernel/i387.c > +++ b/arch/x86/kernel/i387.c > @@ -63,7 +63,7 @@ static inline bool interrupted_user_mode(void) > */ > bool irq_fpu_usable(void) > { > - return !in_interrupt() || > + return !in_serving_interrupt() || > interrupted_user_mode() || > interrupted_kernel_fpu_idle(); > } > diff --git a/include/asm-generic/simd.h b/include/asm-generic/simd.h > index f57eb7b..74e0b05 100644 > --- a/include/asm-generic/simd.h > +++ b/include/asm-generic/simd.h > @@ -10,5 +10,5 @@ > */ > static __must_check inline bool may_use_simd(void) > { > - return !in_interrupt(); > + return !in_serving_interrupt(); > } > diff --git a/include/linux/preempt_mask.h b/include/linux/preempt_mask.h > index dbeec4d..18f3b46 100644 > --- a/include/linux/preempt_mask.h > +++ b/include/linux/preempt_mask.h > @@ -65,6 +65,8 @@ > #define in_softirq() (softirq_count()) > #define in_interrupt() (irq_count()) > #define in_serving_softirq() (softirq_count() & SOFTIRQ_OFFSET) > +#define in_serving_interrupt() (irq_count() & (HARDIRQ_MASK | \ > + SOFTIRQ_OFFSET | NMI_MASK)) > > /* > * Are we in NMI context? > > Thanks, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 14, 2014 at 05:17:05PM +0100, Ard Biesheuvel wrote: > On 14 November 2014 16:43, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > While working on the cryptd request reordering problem, I noticed > > an anomaly where kernel threads are normally allowed to use simd > > per may_use_simd, but as soon as you disable softirqs, they suddenly > > lose that ability for no good reason. > > > > The problem is that in_interrupt does not distinguish between > > softirq processing and simply having softirqs disabled. This > > patch creates a new helper in_serving_interrupt which makes that > > distinction. It then uses this in all current implementations > > of may_use_simd. > > > > Isn't that a much more widespread problem if in_interrupt() yields > true while no interrupt is being served? Good point. Other users of in_interrupt() are just as likely to really want the in_serving version. However, the safe option is to leave them unchanged until we audit each single one. If it turns out that all users of in_interrupt() decay into in_serving_interrupt() then we could remove in_interrupt(). Cheers,
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c index a9a4229..6cdaa852 100644 --- a/arch/x86/kernel/i387.c +++ b/arch/x86/kernel/i387.c @@ -63,7 +63,7 @@ static inline bool interrupted_user_mode(void) */ bool irq_fpu_usable(void) { - return !in_interrupt() || + return !in_serving_interrupt() || interrupted_user_mode() || interrupted_kernel_fpu_idle(); } diff --git a/include/asm-generic/simd.h b/include/asm-generic/simd.h index f57eb7b..74e0b05 100644 --- a/include/asm-generic/simd.h +++ b/include/asm-generic/simd.h @@ -10,5 +10,5 @@ */ static __must_check inline bool may_use_simd(void) { - return !in_interrupt(); + return !in_serving_interrupt(); } diff --git a/include/linux/preempt_mask.h b/include/linux/preempt_mask.h index dbeec4d..18f3b46 100644 --- a/include/linux/preempt_mask.h +++ b/include/linux/preempt_mask.h @@ -65,6 +65,8 @@ #define in_softirq() (softirq_count()) #define in_interrupt() (irq_count()) #define in_serving_softirq() (softirq_count() & SOFTIRQ_OFFSET) +#define in_serving_interrupt() (irq_count() & (HARDIRQ_MASK | \ + SOFTIRQ_OFFSET | NMI_MASK)) /* * Are we in NMI context?
While working on the cryptd request reordering problem, I noticed an anomaly where kernel threads are normally allowed to use simd per may_use_simd, but as soon as you disable softirqs, they suddenly lose that ability for no good reason. The problem is that in_interrupt does not distinguish between softirq processing and simply having softirqs disabled. This patch creates a new helper in_serving_interrupt which makes that distinction. It then uses this in all current implementations of may_use_simd. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Thanks,