Re: [patch 06/11] Use virtual debug registers in process/threadhandling code

[Ingo Molnar]

* Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:

> On Tue, 10 Mar 2009, Ingo Molnar wrote:
> 
> > > @@ -595,6 +596,12 @@ __switch_to(struct task_struct *prev_p, 
> > >  
> > >  	percpu_write(current_task, next_p);
> > >  
> > > +	/*
> > > +	 * Handle debug registers.  This must be done _after_ current
> > > +	 * is updated.
> > > +	 */
> > > +	if (unlikely(test_tsk_thread_flag(next_p, TIF_DEBUG)))
> > > +		switch_to_thread_hw_breakpoint(next_p);
> > 
> > why does this have to be called after 'current' has been 
> > updated? AFAICS switch_to_thread_hw_breakpoint() does not take a 
> > look at 'current'.
> 
> There was a discussion about this on LKML last October 17, and 
> you were in the CC list. [...]

I am on the Cc: list of thousands of messages per month. 
Consider it a very volatile form of storage.

Instead put these:

> There's a problem with moving the 
> switch_to_thread_hw_breakpoint() call before current is 
> updated.  Suppose a kernel breakpoint is triggered in between 
> the two.  The hw-breakpoint handler will see that current is 
> different from the task pointer stored in the chbi area, so it 
> will think the task pointer is leftover from an old task (lazy 
> switching) and will erase it.  Then until the next context 
> switch, no user-breakpoints will be installed.
> 
> The real problem is that it's impossible to update both 
> current and chbi->bp_task at the same instant, so there will 
> always be a window in which they disagree and a breakpoint 
> might get triggered.  Since we use lazy switching, we are 
> forced to assume that a disagreement means that current is 
> correct and chbi->bp_task is old.  But if you move the code 
> above then you'll create a window in which current is old and 
> chbi->bp_task is correct.

inside these:

  /*
   * ......
   */

Thanks,

        Ingo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/