[PATCH v2] um: Enable preemption in UML

[Johannes Berg]

Hi,

Haven't chimed in here before, but thanks for looking at this! I
actually really wanted at least the things you get for debug from this
in the past, so much appreciated.

I think I actually tried a simpler approach and it kind of even worked,
but my userspace likely has very little FPU usage.


> +/* UML knows about 387 features up to and including AVX512, tile, etc are not yet
> + * supported.
> + */

Do you ensure that somehow? Warn if it's there at least? I may have
missed that.

Or is it just for the saving? But what if you don't save enough and the
host CPU has more?


> +void kernel_fpu_begin(void)
> +{
> +#if defined(PREEMPT) || defined(PREEMPT_VOLUNTRARY)

maybe leave it as an inline in case it'd be empty, i.e. neither of these
are defined?

> +	preempt_disable();
> +
> +	WARN_ON(this_cpu_read(in_kernel_fpu));
> +
> +	this_cpu_write(in_kernel_fpu, true);
> +
> +#ifdef CONFIG_64BIT
> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
> +		__builtin_ia32_xsaveopt64(&current->thread.fpu, KNOWN_387_FEATURES);
> +	else {
> +		if (cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE))
> +			__builtin_ia32_xsave64(&current->thread.fpu, KNOWN_387_FEATURES);
> +		else
> +			__builtin_ia32_fxsave64(&current->thread.fpu);
> +	}

nit: you could make all of these else if chains, and avoid the braces:

if (likely(...))
	...
else if (cpu_has(...))
	...
else
	...


> +void kernel_fpu_end(void)

same comments as above apply here.


>  void sigio_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
>  {
> +	preempt_disable();
>  	_sigio_handler(regs, irqs_suspended);
> +	preempt_enable();
> 

I may have to look at those time-travel bits here :)

Thanks,
johannes