On 06/03/15 14:01, Oleg Nesterov wrote: > On 03/06, Ingo Molnar wrote: >> >> * Oleg Nesterov <o...@redhat.com> wrote: >> >>> On 03/06, Ingo Molnar wrote: >>>> >>>> * Oleg Nesterov <o...@redhat.com> wrote: >>>> >>>>> [...] The patch above looks "obviously safe", but perhaps I am >>>>> paranoid too much... >>>> >>>> IMHO your hack above isn't really acceptable, even for a backport. >>>> So lets test the patch below (assuming it's the right thing to do) >>>> and move forward? >>> >>> OK, but please note that this patch is not beckportable. If you think >>> that -stable doesn't need this fix, then I agree. >>> >>> If the caller is do_device_not_available(), then we can not enable >>> irqs before __thread_fpu_begin() + restore_fpu_checking(). >>> >>> 1. Preemption in between can destroy ->fpu.state initialized by >>> fpu_finit(), __switch_to() will save the live (wrong) FPU state >>> again. >>> >>> 2. kernel_fpu_begin() from irq right after __thread_fpu_begin() is >>> not nice too. It will do __save_init_fpu() and this overwrites >>> ->fpu.state too. >>> >>> Starting from v4.0 it does kernel_fpu_disable(), but the older kernels >>> do not. >>> >>> Ingo, this code is really horrible and fragile. We need to cleanup it >>> step-by-step, imho. >> >> How about the patch from David Vrabel? That seems to solve the >> irq-disable problem too, right? > > I wasn't cc'ed, I guess you mean > > [PATCHv4] x86, fpu: remove the logic of non-eager fpu mem allocation at > the first usage > http://marc.info/?l=linux-kernel&m=142564237705311&w=2
This patch is from Suresh, and was originally against 3.10, so... > Not sure I understand it correctly after the first quick look, but > > 1. It conflicts with the recent changes in tip/x86/fpu > > 2. fpu_ini() initializes current->thread.fpu.state. This looks unneeded, > the kernel threads no longer have FPU context and do not abuse CPU. > > 3. I can be easily wrong, but it looks buggy... Note that > arch_dup_task_struct() doesn't allocate child->fpu.state if > !tsk_used_math(parent). ...yes. It's bit-rotted a bit. > No, I do not think this patch is a good idea. Perhaps I am wrong, but I > think we need other changes. And they should start from init_fpu(). But the general principle of avoiding the allocation in the #NM handler and hence avoiding the need to re-enable IRQs is still a good idea, yes? David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
