Re: [RFC PATCH v2] x86/fpu: make kernel-mode FPU reliably usable in softirqs

[Ingo Molnar]

* Eric Biggers <ebiggers@xxxxxxxxxx> wrote:

> [...] To avoid irqs_disabled() entirely, we'd need to avoid disabling 
> softirqs, which would mean supporting nested kernel-mode FPU in 
> softirqs.  I can sent out a patch that does that using a per-CPU 
> buffer, if you'd like to see that.  I wasn't super happy with the 
> extra edge cases and memory usage, but we could go in that direction.

Meh: so I just checked, and local_bh_disable()/enable() are pretty 
heavy these days - it's not just a simple preempt-count twiddle and a 
check anymore. :-/ I don't think my initial argument of irqs_disabled() 
overhead is really valid - and if we really cared we could halve it by 
saving the irqs_disabled() status at kernel_fpu_begin() time and 
reading it at kernel_fpu_end() time.

And the alternative of having nested FPU usage and extra per-CPU FPU 
save areas for the kernel feels a bit fragile, even without having seen 
the patch.

So I think I'll commit your patch to tip:x86/fpu as-is, unless someone 
objects.


BTW., a side note, I was also reviewing the kernel_fpu_begin()/end() 
codepaths, and we have gems like:

        /* Put sane initial values into the control registers. */
        if (likely(kfpu_mask & KFPU_MXCSR) && boot_cpu_has(X86_FEATURE_XMM))
                ldmxcsr(MXCSR_DEFAULT);

        if (unlikely(kfpu_mask & KFPU_387) && boot_cpu_has(X86_FEATURE_FPU))
                asm volatile ("fninit");

has the LDMXCSR instruction, or its effects, ever shown up in profiles?

Because AFAICS these will execute all the time on x86-64, because:

static inline void kernel_fpu_begin(void)
{
#ifdef CONFIG_X86_64
        /*
         * Any 64-bit code that uses 387 instructions must explicitly request
         * KFPU_387.
         */
        kernel_fpu_begin_mask(KFPU_MXCSR);

And X86_FEATURE_XMM is set in pretty much every x86 CPU.

Thanks,

	Ingo