[PATCH] powerpc/32bit,PREEMPT:Load TI_FLAGS to check NEED_RESCHED

[Jain Priyanka-B32167]

If we go some more lines up in the same file, the code is 

resume_kernel:
        /* check current_thread_info, _TIF_EMULATE_STACK_STORE */
        CURRENT_THREAD_INFO(r9, r1)
        lwz     r8,TI_FLAGS(r9)
        andis.  r8,r8,_TIF_EMULATE_STACK_STORE at h
        beq+    1f
---

After execution of andis. Instruction, r8 contains logical AND result of r8 and _TIF_EMULATE_STACK_STORE and it also sets the condition register flag.
If beq+ is true, it will jump to label '1' which points to the code contained in the patch. In this particular scenario, r8 does not contains the appropriate value.

Regards
Priyanka


> -----Original Message-----
> From: tiejun.chen [mailto:tiejun.chen at windriver.com]
> Sent: Monday, May 27, 2013 12:15 PM
> To: Jain Priyanka-B32167
> Cc: linuxppc-dev at lists.ozlabs.org; Wood Scott-B07421
> Subject: Re: [PATCH] powerpc/32bit,PREEMPT:Load TI_FLAGS to check
> NEED_RESCHED
> 
> On 05/27/2013 02:27 PM, Priyanka Jain wrote:
> > Add instruction to load TI_FLAGS in r8
> >
> > While returning from exception handling in case of PREEMPT enabled,
> > _TIF_NEED_RESCHED bit is checked in TI_FLAGS (thread_info flag) of
> > current task. Only if this bit is set, it should continue with the
> > process of calling preempt_schedule_irq() to schedule highest priority
> > task if available.
> >
> > Current code assumes that r8 contains TI_FLAGS and check this for
> > _TIF_NEED_RESCHED, but as r8 is modified in the code which executes
> > before
> 
> Could you elaborate this scenario? As I take a look at this path:
> 
> 	...
>          /* Clear _TIF_EMULATE_STACK_STORE flag */
>          lis     r11,_TIF_EMULATE_STACK_STORE at h
>          addi    r5,r9,TI_FLAGS
> 0:      lwarx   r8,0,r5
>          andc    r8,r8,r11
> #ifdef CONFIG_IBM405_ERR77
>          dcbt    0,r5
> #endif
>          stwcx.  r8,0,r5
>          bne-    0b
> 1:
> 
> #ifdef CONFIG_PREEMPT
>          /* check current_thread_info->preempt_count */
>          lwz     r0,TI_PREEMPT(r9)
>          cmpwi   0,r0,0          /* if non-zero, just restore regs and
> return */
>          bne     restore
>          andi.   r8,r8,_TIF_NEED_RESCHED
> 	...
> 
> Where is R8 clobbered?
> 
> Tiejun
> 
> > this check, r8 no longer contains the expected TI_FLAGS information.
> >
> > As a result check for comparison with _TIF_NEED_RESCHED was failing
> > even if NEED_RESCHED bit is set in the current thread_info flag. Due
> > to this,
> > preempt_schedule_irq() and in turn scheduler was not getting called
> > even if highest priority task is ready for execution.
> >
> >
> > Signed-off-by: Priyanka Jain <Priyanka.Jain at freescale.com>
> > ---
> >   arch/powerpc/kernel/entry_32.S |    1 +
> >   1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/entry_32.S
> > b/arch/powerpc/kernel/entry_32.S index d22e73e..0239c7f 100644
> > --- a/arch/powerpc/kernel/entry_32.S
> > +++ b/arch/powerpc/kernel/entry_32.S
> > @@ -887,6 +887,7 @@ resume_kernel:
> >   #ifdef CONFIG_PREEMPT
> >   	/* check current_thread_info->preempt_count */
> >   	lwz	r0,TI_PREEMPT(r9)
> > +	lwz	r8,TI_FLAGS(r9)
> >   	cmpwi	0,r0,0		/* if non-zero, just restore regs and
> return */
> >   	bne	restore
> >   	andi.	r8,r8,_TIF_NEED_RESCHED
> >
>