[PATCH v7] um: Enable preemption in UML

[Johannes Berg]

> @@ -23,7 +23,7 @@ struct thread_info {
>  	int			preempt_count;  /* 0 => preemptable,
>  						   <0 => BUG */
>  	struct thread_info	*real_thread;    /* Points to non-IRQ stack */
> -	unsigned long aux_fp_regs[FP_SIZE];	/* auxiliary fp_regs to save/restore
> +	unsigned long aux_fp_regs[FP_SIZE] __aligned(64);	/* auxiliary fp_regs to save/restore
>  						   them out-of-band */

nit: that comment looks strange now, maybe pull it up before the member?

 /* auxiliary ... out-of-band */
 unsigned long aux_fp_regs[...] __aligned(64);


> +#ifdef CONFIG_64BIT
> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
> +		__builtin_ia32_xsaveopt64(&current_thread_info()->aux_fp_regs, KNOWN_387_FEATURES);
> +	else {
> +		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> +			__builtin_ia32_xsave64(&current_thread_info()->aux_fp_regs, KNOWN_387_FEATURES);
> +		else
> +			__builtin_ia32_fxsave64(&current_thread_info()->aux_fp_regs);
> +	}

Why not write this as a chain?

	if (likely(cpu_has(...))
		__builtin_ia32_xsaveopt64(...);
	else if (likely(cpu_has(...)))
		__builtin_ia32_xave64(...);
	else
		__builtin_ia32_fxsave64(...);


and IMHO pulling the "&current_thread_info()->aux_fp_regs" that appears
on all three of them into a local variable would make that more readable
too.

> +#else
> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
> +		__builtin_ia32_xsaveopt(&current->aux_fp_regs, KNOWN_387_FEATURES);
> +	else {
> +		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> +			__builtin_ia32_xsave(&current->aux_fp_regs, KNOWN_387_FEATURES);
> +		else
> +			__builtin_ia32_fxsave(&current->aux_fp_regs);
> +	}
> +#endif

And all of the above also applies for 32-bit,

> +void kernel_fpu_end(void)

and this as well.

johannes