Re: [PATCHv2] cobalt/x86: fix condition in eager fpu code for kernels < 4.14

[Mauro Salvini via Xenomai]

On Tue, 2019-01-08 at 18:51 +0100, Henning Schild wrote:
> Am Tue, 8 Jan 2019 15:17:11 +0100
> schrieb Mauro Salvini <mauro.salv...@smigroup.net>:
> 
> > On Mon, 2019-01-07 at 14:04 +0100, Henning Schild wrote:
> > > From: Henning Schild <henning.sch...@siemens.com>
> > > 
> > > 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 <mauro.salv...@smigroup.net>
> > > Signed-off-by: Henning Schild <henning.sch...@siemens.com>
> > > ---
> > >  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(&current->thread.fpu,
> > > smp_processor_id()); #else
> > >   /* mark current thread as not owning the FPU anymore */
> > > - if (&current->thread.fpu.fpstate_active)
> > > + if (fpregs_active())
> > >           fpregs_deactivate(&current->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
> 
>