-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
glibc/hard-float: math/test-fenv-tls sporadic failure #54
Comments
It seems this can happen on ARCv2 too, if rarely: at least one of my HSDK run logs had this FAIL'ing. On ARCv2 FPGA it could be easily triggered when ran in a loop.
|
So the issue is how FPU_STATUS is programed to raise/clear exceptions. Explicit setting of Exception Flag bits [0] thru [4] requires setting FWE [31]. This is what the glibc macros do
However in case of a task switch Linux kernel also needs to switch the FPU_STATUS to incoming task's bits. That code doesn't wiggle the FWE bit like above and thus can fail to effect the update. Here's the excerpt from nSIM trace.
|
When kernel starts a task fpu_init_task() sets the FWE bit once assuming that it is retained. glibc cautiously sets FWE everytime it updates FPU_STATUS so kernel need not do anything special on context switch. However it seems FWE is clear-on-write thus kernel needs to set it even on context switch code. void fpu_save_restore(struct task_struct *prev, struct task_struct *next)
{
struct arc_fpu *save = &prev->thread.fpu;
struct arc_fpu *restore = &next->thread.fpu;
+ const unsigned int fwe = 0x80000000;
save->ctrl = read_aux_reg(ARC_REG_FPU_CTRL);
save->status = read_aux_reg(ARC_REG_FPU_STATUS);
write_aux_reg(ARC_REG_FPU_CTRL, restore->ctrl);
- write_aux_reg(ARC_REG_FPU_STATUS, restore->status);
+ write_aux_reg(ARC_REG_FPU_STATUS, (fwe | restore->status)); |
Local testing confirms the fix. |
…itch FPU_STATUS register contains FP exception flags bits. These can be manually updated (vs. side-effect of FP instructions) by glibc functions fe{raise,clear,test}except() etc. The programming model however requires setting an additional FPU_STATUS.FWE (bit 31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after a write and needs to be set for every update which ARC glibc port does. Linux kernel context switch code swaps the FPU_STATUS value for incoming task also needs to add the FWE bit to effect the write of new value, or else old value will linger around leading to extrmeely hard to debug FP bugs. Fortunately glibc's math/test-fenv-tls recreates this scenario by repeatedy setting/clearing exception flags in a big loop, concurrently in main program and also in a thread, which is how this problem was observed in first place. Fixes: #54 Cc: [email protected] Signed-off-by: Vineet Gupta <[email protected]>
Any FPU_STATUS write needs setting the FWE bit (31) whcih just provides a "control signal" to enable explicit write (vs. the side-effect of FPU instructions). However this bit is RAZ and write-only, thus effectively never stored in FPU_STATUS register. Thus when reading the register there is no need to clear it. This shaves off a BCLR instruction from the fe*exceptino family of functions and while no big deal still makes sense to do. This came up when debugging a race in math/test-fenv-tls [1] [1]: foss-for-synopsys-dwc-arc-processors/linux#54 Signed-off-by: Vineet Gupta <[email protected]>
…itch FPU_STATUS register contains FP exception flags bits which are updated as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit(231). This bit is write-only and RAZ, meaning it is effectively auto-cleared after a write and thus needs to be set everytime which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which however missing the OR'ing with FWE bit, meaning the new value is not effectively being written at all, which is what this patch fixes. This was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small and will end up causing extremely hard to reproduce/debug issues. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] #5.6+ Signed-off-by: Vineet Gupta <[email protected]>
…itch FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] #5.6+ Signed-off-by: Vineet Gupta <[email protected]>
Fix posted to lkml: http://lists.infradead.org/pipermail/linux-snps-arc/2021-July/005247.html |
Any FPU_STATUS write needs setting the FWE bit (31) whcih just provides a "control signal" to enable explicit write (vs. the side-effect of FPU instructions). However this bit is RAZ and write-only, thus effectively never stored in FPU_STATUS register. Thus when reading the register there is no need to clear it. This shaves off a BCLR instruction from the fe*exceptino family of functions and while no big deal still makes sense to do. This came up when debugging a race in math/test-fenv-tls [1] [1]: foss-for-synopsys-dwc-arc-processors/linux#54 Signed-off-by: Vineet Gupta <[email protected]>
…itch FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] #5.6+ Signed-off-by: Vineet Gupta <[email protected]>
…itch FPU_STATUS register contains FP exception flags bits. These can be manually updated (vs. side-effect of FP instructions) by glibc functions fe{raise,clear,test}except() etc. The programming model however requires setting an additional FPU_STATUS.FWE (bit 31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after a write and needs to be set for every update which ARC glibc port does. Linux kernel context switch code swaps the FPU_STATUS value for incoming task also needs to add the FWE bit to effect the write of new value, or else old value will linger around leading to extrmeely hard to debug FP bugs. Fortunately glibc's math/test-fenv-tls recreates this scenario by repeatedy setting/clearing exception flags in a big loop, concurrently in main program and also in a thread, which is how this problem was observed in first place. Fixes: #54 Cc: [email protected] Signed-off-by: Vineet Gupta <[email protected]>
Fix merged in mainline |
…itch commit 3a715e8 upstream. FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors/linux#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] #5.6+ Signed-off-by: Vineet Gupta <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
…itch stable inclusion from stable-5.10.60 commit ca6dea44bd8cf953a11866fd63d1a5fd9eec81a9 bugzilla: 177018 https://gitee.com/openeuler/kernel/issues/I4EAUG Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=ca6dea44bd8cf953a11866fd63d1a5fd9eec81a9 -------------------------------- commit 3a715e8 upstream. FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors/linux#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] #5.6+ Signed-off-by: Vineet Gupta <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Chen Jun <[email protected]> Acked-by: Weilong Chen <[email protected]> Signed-off-by: Chen Jun <[email protected]> Signed-off-by: Zheng Zengkai <[email protected]>
…itch BugLink: https://bugs.launchpad.net/bugs/1943756 commit 3a715e8 upstream. FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors/linux#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] #5.6+ Signed-off-by: Vineet Gupta <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Kamal Mostafa <[email protected]> Signed-off-by: Stefan Bader <[email protected]>
…itch FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors/linux#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] csi2115-super#5.6+ Signed-off-by: Vineet Gupta <[email protected]>
…itch FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] #5.6+ Signed-off-by: Vineet Gupta <[email protected]>
…itch commit 3a715e8 upstream. FPU_STATUS register contains FP exception flags bits which are updated by core as side-effect of FP instructions but can also be manually wiggled such as by glibc C99 functions fe{raise,clear,test}except() etc. To effect the update, the programming model requires OR'ing FWE bit (31). This bit is write-only and RAZ, meaning it is effectively auto-cleared after write and thus needs to be set everytime: which is how glibc implements this. However there's another usecase of FPU_STATUS update, at the time of Linux task switch when incoming task value needs to be programmed into the register. This was added as part of f45ba2b ("ARCv2: fpu: preserve userspace fpu state") which missed OR'ing FWE bit, meaning the new value is effectively not being written at all. This patch remedies that. Interestingly, this snafu was not caught in interm glibc testing as the race window which relies on a specific exception bit to be set/clear is really small specially when it nvolves context switch. Fortunately this was caught by glibc's math/test-fenv-tls test which repeatedly set/clear exception flags in a big loop, concurrently in main program and also in a thread. Fixes: foss-for-synopsys-dwc-arc-processors/linux#54 Fixes: f45ba2b ("ARCv2: fpu: preserve userspace fpu state") Cc: [email protected] gregkh#5.6+ Signed-off-by: Vineet Gupta <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
When testing arc64 glibc hf support (on nSIM/FPGA) I observed sporadic failure of math/test-fenv-tls
It seems the issue was always FP exceptions related (test also checks rounding modes etc which works fine). So created a simple test.
This test would always passes.
Next as in original test, adding a thread doing this (in addition to main thread) and was able to hit the problem.
The issue turned out to be FPU_STATUS register which is poked by feraiseexcept / feclearexcept / fetestexcept was not retaining the right value across Linux task switch. This is despite CONFIG_ARC_FPU_SAVE_RESTORE=y in kernel.
The text was updated successfully, but these errors were encountered: