• Peter Zijlstra's avatar
    sched: Fix race against ptrace_freeze_trace() · d136122f
    Peter Zijlstra authored
    There is apparently one site that violates the rule that only current
    and ttwu() will modify task->state, namely ptrace_{,un}freeze_traced()
    will change task->state for a remote task.
    
    Oleg explains:
    
      "TASK_TRACED/TASK_STOPPED was always protected by siglock. In
    particular, ttwu(__TASK_TRACED) must be always called with siglock
    held. That is why ptrace_freeze_traced() assumes it can safely do
    s/TASK_TRACED/__TASK_TRACED/ under spin_lock(siglock)."
    
    This breaks the ordering scheme introduced by commit:
    
      dbfb089d ("sched: Fix loadavg accounting race")
    
    Specifically, the reload not matching no longer implies we don't have
    to block.
    
    Simply things by noting that what we need is a LOAD->STORE ordering
    and this can be provided by a control dependency.
    
    So replace:
    
    	prev_state = prev->state;
    	raw_spin_lock(&rq->lock);
    	smp_mb__after_spinlock(); /* SMP-MB */
    	if (... && prev_state && prev_state == prev->state)
    		deactivate_task();
    
    with:
    
    	prev_state = prev->state;
    	if (... && prev_state) /* CTRL-DEP */
    		deactivate_task();
    
    Since that already implies the 'prev->state' load must be complete
    before allowing the 'prev->on_rq = 0' store to become visible.
    
    Fixes: dbfb089d ("sched: Fix loadavg accounting race")
    Reported-by: default avatarJiri Slaby <jirislaby@kernel.org>
    Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
    Acked-by: default avatarOleg Nesterov <oleg@redhat.com>
    Tested-by: default avatarPaul Gortmaker <paul.gortmaker@windriver.com>
    Tested-by: default avatarChristian Brauner <christian.brauner@ubuntu.com>
    d136122f
core.c 203 KB