Source-Changes-D archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src/sys/arch/x86/x86
Le 22/06/2018 à 03:40, matthew green a écrit :
"Maxime Villard" writes:
Module Name: src
Committed By: maxv
Date: Tue Jun 19 09:25:13 UTC 2018
Modified Files:
src/sys/arch/x86/x86: fpu.c
Log Message:
When using EagerFPU, create the fpu state in execve at IPL_HIGH.
why splhigh instead of kpreempt_disable()?
Nick Hudson asked me the same question yesterday, here's the relevant part
of my answer (quoting his mail, I guess he doesn't mind).
The fpu code already runs at splhigh, and this disables preemption. So I
used splhigh too.
-------- Message transféré --------
Sujet : Re: Fwd: CVS commit: src/sys/arch/x86/x86
Date : Thu, 21 Jun 2018 07:23:33 +0100
De : Nick Hudson <nick.hudson%gmx.co.uk@localhost>
Pour : Maxime Villard <max%m00nbsd.net@localhost>
On 21/06/2018 07:21, Maxime Villard wrote:
Le 21/06/2018 à 08:11, Nick Hudson a écrit :
On 21/06/2018 06:42, Maxime Villard wrote:
Le 21/06/2018 à 07:34, Maxime Villard a écrit :
Le 21/06/2018 à 07:21, Nick Hudson a écrit :
Surely, kpreempt_{disable,enable}() is what you really mean?
Nick
No, I mean splhigh. splhigh does prevent preemption, is that
incorrect?
If you want to disable preemption then kpreempt_disable() is what you
want.
If you want to disable interrupts (and as a consequence pre-emption)
then splhigh is what you want.
I firmly believe you want the former.
Why disable interupts unnecessarily? This is "bad practice" and
shouldn't be left for others to cargo cult copy.
There is a KASSERT in fpusave_cpu(), which I didn't introduce. We want
IPL_HIGH, unconditionally.
So no, I don't want the former, the FPU code wants the latter. Then we
can say that maybe splhigh is not needed after all in fpusave_cpu; I won't
risk introducing random races.
In the commit I said "disable preemption" because I had in mind the
problem where a context switch occurs as a result of preemption.
Up to you... really don't know why FPU code needs interrupts disabled.
Home |
Main Index |
Thread Index |
Old Index