Re: [PATCHv2] cobalt/x86: fix condition in eager fpu code for kernels < 4.14
On 07.01.19 21:04, Henning Schild via Xenomai wrote: From: Henning Schild We should mark the current task as not owning the fpu anymore if it does actually own the fpu, not if the fpu itself is active. Fixes cb52e6c7438fa Reported-by: Mauro Salvini Signed-off-by: Henning Schild --- kernel/cobalt/arch/x86/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/cobalt/arch/x86/thread.c b/kernel/cobalt/arch/x86/thread.c index a09560075..ba807ac1e 100644 --- a/kernel/cobalt/arch/x86/thread.c +++ b/kernel/cobalt/arch/x86/thread.c @@ -475,7 +475,7 @@ void xnarch_leave_root(struct xnthread *root) switch_fpu_finish(>thread.fpu, smp_processor_id()); #else /* mark current thread as not owning the FPU anymore */ - if (>thread.fpu.fpstate_active) + if (fpregs_active()) fpregs_deactivate(>thread.fpu); #endif } Thanks, applied it next. Jan
Re: [PATCHv2] cobalt/x86: fix condition in eager fpu code for kernels < 4.14
On Tue, 2019-01-08 at 18:51 +0100, Henning Schild wrote: > Am Tue, 8 Jan 2019 15:17:11 +0100 > schrieb Mauro Salvini : > > > On Mon, 2019-01-07 at 14:04 +0100, Henning Schild wrote: > > > From: Henning Schild > > > > > > We should mark the current task as not owning the fpu anymore if > > > it > > > does > > > actually own the fpu, not if the fpu itself is active. > > > > > > Fixes cb52e6c7438fa > > > > > > Reported-by: Mauro Salvini > > > Signed-off-by: Henning Schild > > > --- > > > kernel/cobalt/arch/x86/thread.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/kernel/cobalt/arch/x86/thread.c > > > b/kernel/cobalt/arch/x86/thread.c > > > index a09560075..ba807ac1e 100644 > > > --- a/kernel/cobalt/arch/x86/thread.c > > > +++ b/kernel/cobalt/arch/x86/thread.c > > > @@ -475,7 +475,7 @@ void xnarch_leave_root(struct xnthread *root) > > > switch_fpu_finish(>thread.fpu, > > > smp_processor_id()); #else > > > /* mark current thread as not owning the FPU anymore */ > > > - if (>thread.fpu.fpstate_active) > > > + if (fpregs_active()) > > > fpregs_deactivate(>thread.fpu); > > > #endif > > > } > > > > Hi all, > > > > I tried to launch xeno-test several times under same previous > > conditions and I confirm that this patch fixes the bug. > > Good to hear, thanks! > > > A side note: would not #include in > > kernel/cobalt/arch/x86/thread.c lines 47,48 and 54 be redundant > > with > > same includes in > > kernel/cobalt/arch/x86/include/asm/xenomai/thread.h > > (that is indirectly included in thread.c)? > > The latter does not have those includes. It is still very possible > that there are a few too many but that is what you have inclusion > guards > for. And the code you are looking at 47,48 and 54 is all to support > legacy/old kernels. You are in the IPIPE_X86_FPU_EAGER case. Yes, sorry, I wanted to write kernel/cobalt/arch/x86/include/asm/xenomai/wrappers.h Anyway no problem to have them in both files. Regards Mauro > > Henning > > > Thanks, regards > > Mauro > >
Re: [PATCHv2] cobalt/x86: fix condition in eager fpu code for kernels < 4.14
Am Tue, 8 Jan 2019 15:17:11 +0100 schrieb Mauro Salvini : > On Mon, 2019-01-07 at 14:04 +0100, Henning Schild wrote: > > From: Henning Schild > > > > We should mark the current task as not owning the fpu anymore if it > > does > > actually own the fpu, not if the fpu itself is active. > > > > Fixes cb52e6c7438fa > > > > Reported-by: Mauro Salvini > > Signed-off-by: Henning Schild > > --- > > kernel/cobalt/arch/x86/thread.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/cobalt/arch/x86/thread.c > > b/kernel/cobalt/arch/x86/thread.c > > index a09560075..ba807ac1e 100644 > > --- a/kernel/cobalt/arch/x86/thread.c > > +++ b/kernel/cobalt/arch/x86/thread.c > > @@ -475,7 +475,7 @@ void xnarch_leave_root(struct xnthread *root) > > switch_fpu_finish(>thread.fpu, > > smp_processor_id()); #else > > /* mark current thread as not owning the FPU anymore */ > > - if (>thread.fpu.fpstate_active) > > + if (fpregs_active()) > > fpregs_deactivate(>thread.fpu); > > #endif > > } > > Hi all, > > I tried to launch xeno-test several times under same previous > conditions and I confirm that this patch fixes the bug. Good to hear, thanks! > A side note: would not #include in > kernel/cobalt/arch/x86/thread.c lines 47,48 and 54 be redundant with > same includes in kernel/cobalt/arch/x86/include/asm/xenomai/thread.h > (that is indirectly included in thread.c)? The latter does not have those includes. It is still very possible that there are a few too many but that is what you have inclusion guards for. And the code you are looking at 47,48 and 54 is all to support legacy/old kernels. You are in the IPIPE_X86_FPU_EAGER case. Henning > Thanks, regards > Mauro
Re: [PATCHv2] cobalt/x86: fix condition in eager fpu code for kernels < 4.14
On Mon, 2019-01-07 at 14:04 +0100, Henning Schild wrote: > From: Henning Schild > > We should mark the current task as not owning the fpu anymore if it > does > actually own the fpu, not if the fpu itself is active. > > Fixes cb52e6c7438fa > > Reported-by: Mauro Salvini > Signed-off-by: Henning Schild > --- > kernel/cobalt/arch/x86/thread.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/cobalt/arch/x86/thread.c > b/kernel/cobalt/arch/x86/thread.c > index a09560075..ba807ac1e 100644 > --- a/kernel/cobalt/arch/x86/thread.c > +++ b/kernel/cobalt/arch/x86/thread.c > @@ -475,7 +475,7 @@ void xnarch_leave_root(struct xnthread *root) > switch_fpu_finish(>thread.fpu, smp_processor_id()); > #else > /* mark current thread as not owning the FPU anymore */ > - if (>thread.fpu.fpstate_active) > + if (fpregs_active()) > fpregs_deactivate(>thread.fpu); > #endif > } Hi all, I tried to launch xeno-test several times under same previous conditions and I confirm that this patch fixes the bug. A side note: would not #include in kernel/cobalt/arch/x86/thread.c lines 47,48 and 54 be redundant with same includes in kernel/cobalt/arch/x86/include/asm/xenomai/thread.h (that is indirectly included in thread.c)? Thanks, regards Mauro
