Re: [BUG?][2.6.25-mm1] sleeping during IRQ disabled

[Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>]

Hi all,

Luck, Tony wrote:
>> So it's definitely in mainline, and its definitely
>> not Seto-san's patch.
> 
> Here's the root of the problem (arch/ia64/kernel/entry.S):
> 

Please see:

    841 GLOBAL_ENTRY(ia64_leave_kernel)
    842         PT_REGS_UNWIND_INFO(0)
    843         /*
    844          * work.need_resched etc. mustn't get changed by this CPU before it returns to
    845          * user- or fsys-mode, hence we disable interrupts early on.
    846          *
    847          * p6 controls whether current_thread_info()->flags needs to be check for
    848          * extra work.  We always check for extra work when returning to user-level.
    849          * With CONFIG_PREEMPT, we also check for extra work when the preempt_count
    850          * is 0.  After extra work processing has been completed, execution
    851          * resumes at .work_processed_syscall with p6 set to 1 if the extra-work-check
    852          * needs to be redone.
    853          */
    854 #ifdef CONFIG_PREEMPT
    855         rsm psr.i                               // disable interrupts
    856         cmp.eq p0,pLvSys=r0,r0                  // pLvSys=0: leave from
    856 kernel
    857 (pKStk) adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
    858         ;;
    859         .pred.rel.mutex pUStk,pKStk
    860 (pKStk) ld4 r21=[r20]                   // r21 <- preempt_count
    861 (pUStk) mov r21=0                       // r21 <- 0
    862         ;;
    863         cmp.eq p6,p0=r21,r0             // p6 <- pUStk || (preempt_count
    863  = 0)
    864 #else
    865 (pUStk) rsm psr.i
    866         cmp.eq p0,pLvSys=r0,r0          // pLvSys=0: leave from kernel
    867 (pUStk) cmp.eq.unc p6,p0=r0,r0          // p6 <- pUStk
    868 #endif
    869 .work_processed_kernel:
    870         adds r17=TI_FLAGS+IA64_TASK_SIZE,r13
    871         ;;
    872 (p6)    ld4 r31=[r17]                           // load current_thread_i
    872 nfo()->flags
    873         adds r21=PT(PR)+16,r12
    874         ;;
    875
    876         lfetch [r21],PT(CR_IPSR)-PT(PR)
    877         adds r2=PT(B6)+16,r12
    878         adds r3=PT(R16)+16,r12
    879         ;;
    880         lfetch [r21]
    881         ld8 r28=[r2],8          // load b6
    882         adds r29=PT(R24)+16,r12
    883
    884         ld8.fill r16=[r3],PT(AR_CSD)-PT(R16)
    885         adds r30=PT(AR_CCV)+16,r12
    886 (p6)    and r19=TIF_WORK_MASK,r31               // any work other than T
    886 IF_SYSCALL_TRACE?
    887         ;;
    888         ld8.fill r24=[r29]
    889         ld8 r15=[r30]           // load ar.ccv
    890 (p6)    cmp4.ne.unc p6,p0=r19, r0               // any special work pend
    890 ing?
    891         ;;
    892         ld8 r29=[r2],16         // load b7
    893         ld8 r30=[r3],16         // load ar.csd
    894 (p6)    br.cond.spnt .work_pending

and:

   1160 .work_pending_syscall:
   1161         add r2=-8,r2
   1162         add r3=-8,r3
   1163         ;;
   1164         st8 [r2]=r8
   1165         st8 [r3]=r10
   1166 .work_pending:
   1167         tbit.z p6,p0=r31,TIF_NEED_RESCHED               // current_thread_info()->need_resched=0?
   1168 (p6)    br.cond.sptk.few .notify
   1169 #ifdef CONFIG_PREEMPT
   1170 (pKStk) dep r21=-1,r0,PREEMPT_ACTIVE_BIT,1
   1171         ;;
   1172 (pKStk) st4 [r20]=r21
   1173         ssm psr.i               // enable interrupts
   1174 #endif
>    1175         br.call.spnt.many rp=schedule
>    1176 .ret9:  cmp.eq p6,p0=r0,r0                              // p6 <- 1
>    1177         rsm psr.i               // disable interrupts
>    1178         ;;
>    1179 #ifdef CONFIG_PREEMPT
>    1180 (pKStk) adds r20=TI_PRE_COUNT+IA64_TASK_SIZE,r13
>    1181         ;;
>    1182 (pKStk) st4 [r20]=r0            // preempt_count() <- 0
>    1183 #endif
>    1184 (pLvSys)br.cond.sptk.few  .work_pending_syscall_end
>    1185         br.cond.sptk.many .work_processed_kernel        // re-check
>    1186 
>    1187 .notify:
>    1188 (pUStk) br.call.spnt.many rp=notify_resume_user
> 
> on line 1188 we call notify_resume_user() with interrupts disabled (at
> least if we fall through from the code above ... I didn't check the
> state of interrupts if we branch to ".notify").

AFAIK, we always call notify_resume_user() with interrupts disabled.
Is this right?

> So we start down this call chain to the might_sleep() check:
> 
>  [<a000000100011bf0>] show_stack+0x50/0xa0
>  [<a000000100011c70>] dump_stack+0x30/0x60
>  [<a000000100061a90>] __might_sleep+0x1f0/0x220
>  [<a000000100709020>] down_read+0x20/0x60
>  [<a0000001000fe060>] access_process_vm+0x60/0x2c0
>  [<a00000010002a280>] ia64_sync_kernel_rbs+0x40/0x100
>  [<a00000010002a400>] do_sync_rbs+0xc0/0x100
>  [<a00000010000ac30>] unw_init_running+0x70/0xa0
>  [<a00000010002a5c0>] ia64_sync_krbs+0x80/0xa0
>  [<a000000100012690>] do_notify_resume_user+0x110/0x140
>  [<a00000010000aac0>] notify_resume_user+0x40/0x60
>  [<a00000010000a9f0>] skip_rbs_switch+0xe0/0x110
>  [<a000000000010740>] __kernel_syscall_via_break+0x0/0x20

So, I think the problem is not "why interrupts are disabled," but
"why sleep in this path which always with interrupts disabled."

It obviously means ia64_sync_kernel_rbs should care about that.
The function was introduced by the following commit:

> commit 3b2ce0b17824c42bc2e46f7dd903b4acf5e9fff9
> Author: Petr Tesarik <ptesarik@suse.cz>
> Date:   Wed Dec 12 15:23:34 2007 +0100
>
>     [IA64] Synchronize kernel RSE to user-space and back

Hmm, could you make ia64_sync_kernel_rbs to safe with interrupts
disabled, Petr?

Thanks,
H.Seto