Re: [patch][RFC] an early patch for x86_64 preempt-rt kernel cause more __might_sleep warnings

Re: [patch][RFC] an early patch for x86_64 preempt-rt kernel cause more __might_sleep warnings

[hyl]

2008/6/30 John Blackwood <john.blackwood@ccur.com>:
>> Subject: Re: [patch][RFC] an early patch for x86_64 preempt-rt kernel
>> cause more __might_sleep warnings
>> From: hyl <heyongli@gmail.com>
>> Date: Wed, 25 Jun 2008 22:38:50 -0400
>> To: "Blackwood, John" <john.blackwood@ccur.com>
>>
>> > > ------------------------------ ------------------------------
>> > >
>> > > Subject: Re: your x86_64 IST stack preempt rt patch
>> > > From: Andi Kleen <andi@firstfloor.org>
>> > > Date: Wed, 25 Jun 2008 03:36:33 -0400
>> > > To: "Blackwood, John" <john.blackwood@ccur.com>
>> > >
>> > >
>> > > This patch changes these traps instead to always switch
>> > > to the process stack when the trap originated from user mode.
>> > > For kernel traps it keeps running non preemptive on the IST stack
>> > > because that is much safer (e.g. to still get nmi watchdog events
>> > > out even when the process stack is corrupted)
>> > >
>> > > Strictly the change would have been only needed for debug
>> > > and int3, but since they share this code as macros it was
>> > > cleanest to just change all.
>> > >
>> > > Additional (small) benefit is less preemption latency in these
>> > > exception cases. This will also likely help preempt-RT kernels a bit
>> > > because those are more likely to take sleeping locks even in these
>> > > cases.
>> > >
>>
>> But with this fix, we still run do_debug/.. on IST when the 'interrupt'
>> occur from kernel space, so the warning message will come out again,
>> right?
>
> that seems correct to me.
>
>> but i don't have a scenario  to proof this.
>
> i think that using something like kgdb or kdb kernel debuggers
> would end up causing this situation.
>
> or using ptrace from user space to set up a debug register to point
> to some kernel address as a breakpoint or watchpoint (this wouldn't be
> something that people would usually want to do though).
  yeah, but there is no patch try to fix it, do you found one ?
>
>
>
>
>

[patch][RFC] an early patch for x86_64 preempt-rt kernel cause more __might_sleep warnings

[hyl]

Hi, John

[Sorry resend this mail, with pure text format and CC to Linux-kernel. ]

I saw you submitted a patch for X86_64 to prevent a oops, here is the
hyper-link i google:
http://marc.info/?l=git-commits-head&m=113979973412652&w=2

There is an obvious problem for your patch , see my comment at  "=====>"

/* runs on IST stack. */
asmlinkage void __kprobes do_debug(struct pt_regs * regs,
                   unsigned long error_code)
{

- conditional_sti(regs);
+ preempt_conditional_sti(regs);


       force_sig_info(SIGTRAP, &info, tsk);=====> /*this function try
spin_lock_irqsave(&t->sighand->siglock, flags); generate more
warnings*/


clear_dr7:
set_debugreg(0UL, 7);
+ preempt_conditional_cli(regs);
return;

clear_TF_reenable:
set_tsk_thread_flag(tsk, TIF_SINGLESTEP);
regs->eflags &= ~TF_MASK;
+ preempt_conditional_cli(regs);

}


here is the trace result, founded on linux2.6.25.4 with realtime
patch(not only do_debug, do_int3,do_stack_segment also has same
problem ):

BUG: sleeping function called from invalid context
usermode-agent(2302) at kernel/rtmutex.c:626

in_atomic():1 [00000001], irqs_disabled():0

Call Trace:
<#DB>  [<ffffffff80227177>] __might_sleep+0x107/0x110
[<ffffffff804ec510>] __rt_spin_lock+0x30/0x60
[<ffffffff804ec549>] rt_spin_lock+0x9/0x10

[<ffffffff8023cf2f>] force_sig_info+0x2f/0xd0
[<ffffffff8020cdab>] do_debug+0x17b/0x220
[<ffffffff804ed1b3>] debug+0x93/0x9f
<<EOE>>

Posssible way to resolve such problem:

1. relpace task->sighand->siglock  as raw_spin_lock_t
    but this will  reduce the preemptive kernel code size

2. if possible, call  preempt_conditional_cli before call force_sig_info
    but we must sure the John's bug not be re-introduce by such action

please don't hesitate to tell me the best solution if you got the good idea.


Best Wishes
hyl

Re: [patch][RFC] an early patch for x86_64 preempt-rt kernel cause more __might_sleep warnings

[hyl]

2008/6/24 hyl <heyongli@gmail.com>:
> Hi, John
>
> [Sorry resend this mail, with pure text format and CC to Linux-kernel. ]
>
> I saw you submitted a patch for X86_64 to prevent a oops, here is the
> hyper-link i google:
> http://marc.info/?l=git-commits-head&m=113979973412652&w=2
>
> There is an obvious problem for your patch , see my comment at  "=====>"
>
> /* runs on IST stack. */
> asmlinkage void __kprobes do_debug(struct pt_regs * regs,
>                   unsigned long error_code)
> {
>
> - conditional_sti(regs);
> + preempt_conditional_sti(regs);
>
>
>       force_sig_info(SIGTRAP, &info, tsk);=====> /*this function try
> spin_lock_irqsave(&t->sighand->siglock, flags); generate more
> warnings*/
>
>
> clear_dr7:
> set_debugreg(0UL, 7);
> + preempt_conditional_cli(regs);
> return;
>
> clear_TF_reenable:
> set_tsk_thread_flag(tsk, TIF_SINGLESTEP);
> regs->eflags &= ~TF_MASK;
> + preempt_conditional_cli(regs);
>
> }
>
>
> here is the trace result, founded on linux2.6.25.4 with realtime
> patch(not only do_debug, do_int3,do_stack_segment also has same
> problem ):
>
> BUG: sleeping function called from invalid context
> usermode-agent(2302) at kernel/rtmutex.c:626
>
> in_atomic():1 [00000001], irqs_disabled():0
>
> Call Trace:
> <#DB>  [<ffffffff80227177>] __might_sleep+0x107/0x110
> [<ffffffff804ec510>] __rt_spin_lock+0x30/0x60
> [<ffffffff804ec549>] rt_spin_lock+0x9/0x10
>
> [<ffffffff8023cf2f>] force_sig_info+0x2f/0xd0
> [<ffffffff8020cdab>] do_debug+0x17b/0x220
> [<ffffffff804ed1b3>] debug+0x93/0x9f
> <<EOE>>

  rt-6 have fix this by disable IST when RT patches is enabled on
x86-64 platform.
sorry, i does not notice this ... :)
>
> Posssible way to resolve such problem:
>
> 1. relpace task->sighand->siglock  as raw_spin_lock_t
>    but this will  reduce the preemptive kernel code size
>
> 2. if possible, call  preempt_conditional_cli before call force_sig_info
>    but we must sure the John's bug not be re-introduce by such action
>
> please don't hesitate to tell me the best solution if you got the good idea.
>
>
> Best Wishes
> hyl
>