Re: Q: user_enable_single_step() && update_debugctlmsr()

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 08/01/2012 05:14 PM, Oleg Nesterov wrote:
> On 08/01, Sebastian Andrzej Siewior wrote:
>>
>> On 08/01/2012 05:01 PM, Oleg Nesterov wrote:
>>> On 08/01, Sebastian Andrzej Siewior wrote:
>>>> So a patch like
>>>> --- a/arch/x86/kernel/step.c
>>>> +++ b/arch/x86/kernel/step.c
>>>> @@ -173,8 +173,8 @@ static void enable_step(struct task_struct *child,
>>>> bool block)
>>>>                   unsigned long debugctl = get_debugctlmsr();
>>>>
>>>>                   debugctl |= DEBUGCTLMSR_BTF;
>>>> -               update_debugctlmsr(debugctl);
>>>>                   set_tsk_thread_flag(child, TIF_BLOCKSTEP);
>>>> +               update_debugctlmsr(debugctl);
>>>>           } else if (test_tsk_thread_flag(child, TIF_BLOCKSTEP)) {
>>>>                   unsigned long debugctl = get_debugctlmsr();
>>>>
>>>> should fix the race
>>>
>>> No, I don't think it can fix something ;) or make any difference.
>>
>> Why? You _first_ set the task flag
>
> Yes, and this task is "child".
>
>> followed by the CPU register. Now
>> switch_to() would see the bit set and act.
>
> child sleeps and doesn't participate in switch_to(). Debugger and another
> (unrelated) task do.

This is confusing.

In order to allow the debugger to ptrace()->enable_blockstep() the
child has to be stopped/traced. We switch X86_EFLAGS_TF in child's regs
and enable DEBUGCTLMSR_BTF for the debugger which is wrong. If we quit
to userspace then the CPU on which the debugger runs has
DEBUGCTLMSR_BTF. If the tracee task runs on the same then nothing 
happens, the bit remains set.
If the tracee happens to run on a different CPU then switch_to() will
enable the DEBUGCTLMSR_BTF bit for the debugger's CPU and switch_to() 
will enable it also on the other CPU.
I added a few printks in the source and I see output that
__switch_to_xtra() enables the bit as well as in enable_single() for
debugger's CPU. I didn't find where the single step is disabled for the
tracee. I haven't notice this in __switch_to_xtra() nor or in
disable_single_step().

> Oleg.
>

Sebastian