Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 Mar 2014 00:02:46 -0700
From:      Julian Elischer <julian@freebsd.org>
To:        freebsd-hackers@freebsd.org, Chris Torek <torek@torek.net>, Konstantin Belousov <kib@FreeBSD.org>
Subject:   Re: slight problem with r254457 (kthread_add fix)
Message-ID:  <53351E96.1000608@freebsd.org>
In-Reply-To: <201403271942.s2RJg2hQ069598@elf.torek.net>
References:  <201403271942.s2RJg2hQ069598@elf.torek.net>

next in thread | previous in thread | raw e-mail | index | archive | help
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@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
>




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53351E96.1000608>