slight problem with r254457 (kthread_add fix)

[Julian Elischer]

On 3/27/14, 12:42 PM, Chris Torek wrote:
>> I agree with the problem statement, but disagree with the proposed
>> 'fix'. I.e., I agree that on the creation of a new kernel thread, the
>> current thread FPU grab state must not leak into the created thread.
>>
>> In fact, cpu_set_upcall() already almost handles this correctly, by
>> resetting the pcb_save pointer back to the user save area.  What is
>> not done is the clearing of PCB_KERNFPU flag, which seems to be the
>> issue you met.  Am I right ?
> Yes.  I considered your fix as well but decided it required also
> understanding other architectures too much. :-)  (I don't know
> if sparc has similar code, for instance, but it *could*; its
> FPU is much like the amd64 one.  You've fixed i386, but there
> are more.)
>
>> If yes, then the following patch should handle the problem, hopefully.
> I'm OK with this too, as long as all architectures do it, or
> something sufficiently similar.

this is the reason that I originally made the new thread 'fork' off an old
one (back in 2000 I think it was.. I was in Budapest I remember.),
anyway I didn't want to be bothered with the internal contents of stuff
I didn't understand on processors I didn't know, and the rule
was "no FPU in the Kernel", so it was "safe" (ish).

I did make a mental note that if there were ever problems with this then
we should write a machine dependent function to set up a new thread 
context,
but I was in a hurry so I did it in the most generic way possible. In 
any case,
fork() had been doing it that way for years too. SO,
I guess the message is:
"no magic was hidden here. any obvious fix you can see is probably 
good enough"
BTW for kernel threads with no proc, My memory is that I was forking 
from thread0,
which was the thread that went on to be running init(). I THINK that 
is still true
but either way, it was not a thread that was going anywhere, and it 
was guaranteed to not
have a floating point operation.
> (Another way to look at this is that creating a new kernel thread
> requires cloning a child from somewhere, but we don't have a
> particularly good "somewhere" so we clone something chosen at what
> amounts to random.  We then have to make this look like a "virgin
> birth", untainted by any previous actions of the thing we're
> cloning.
>
> While I was hunting this problem I also looked at cpu_throw in
> amd64/amd64/cpu_switch.S.  It took me a long time to figure out
> why it is safe to ignore the per-cpu fpcurthread there.  It is,
> because cpu_throw is used for only one case, the "virgin birth"
> setup on each CPU.  There can be no existing thread using the FPU
> at that point, so this is OK.)
>
> Chris
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"
>