Messages in this thread Patch in this message | | | Date | Thu, 21 Apr 2022 22:07:21 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2 5/5] freezer,sched: Rewrite core freezer logic |
| |
On Thu, Apr 21, 2022 at 09:55:51PM +0200, Peter Zijlstra wrote: > On Thu, Apr 21, 2022 at 12:26:44PM -0500, Eric W. Biederman wrote: > > Peter Zijlstra <peterz@infradead.org> writes: > > > > > --- a/kernel/ptrace.c > > > +++ b/kernel/ptrace.c > > > @@ -288,7 +288,7 @@ static int ptrace_check_attach(struct ta > > > } > > > __set_current_state(TASK_RUNNING); > > > > > > - if (!wait_task_inactive(child, TASK_TRACED) || > > > + if (!wait_task_inactive(child, TASK_TRACED|TASK_FREEZABLE) || > > > !ptrace_freeze_traced(child)) > > > return -ESRCH; > > > > Do we mind that this is going to fail if the child is frozen > > during ptrace_check_attach? > > Why should this fail? wait_task_inactive() will in fact succeed if it is > frozen due to the added TASK_FREEZABLE and some wait_task_inactive() > changes elsewhere in this patch.
These:
--- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -3260,6 +3260,19 @@ int migrate_swap(struct task_struct *cur } #endif /* CONFIG_NUMA_BALANCING */ +static inline bool __wti_match(struct task_struct *p, unsigned int match_state) +{ + unsigned int state = READ_ONCE(p->__state); + + if ((match_state & TASK_FREEZABLE) && state == TASK_FROZEN) + return true; + + if (state == (match_state & ~TASK_FREEZABLE)) + return true; + + return false; +} + /* * wait_task_inactive - wait for a thread to unschedule. * @@ -3304,7 +3317,7 @@ unsigned long wait_task_inactive(struct * is actually now running somewhere else! */ while (task_running(rq, p)) { - if (match_state && unlikely(READ_ONCE(p->__state) != match_state)) + if (match_state && !__wti_match(p, match_state)) return 0; cpu_relax(); } @@ -3319,7 +3332,7 @@ unsigned long wait_task_inactive(struct running = task_running(rq, p); queued = task_on_rq_queued(p); ncsw = 0; - if (!match_state || READ_ONCE(p->__state) == match_state) + if (!match_state || __wti_match(p, match_state)) ncsw = p->nvcsw | LONG_MIN; /* sets MSB */ task_rq_unlock(rq, p, &rf);
> And I don't see why ptrace_freeze_traced() should fail. It'll warn > though, I should extend/remove that WARN_ON_ONCE() looking at __state, > but it should work.
And that looks like (after removal of the one WARN):
static bool ptrace_freeze_traced(struct task_struct *task) { unsigned long flags; bool ret = false; /* Lockless, nobody but us can set this flag */ if (task->jobctl & JOBCTL_LISTENING) return ret; if (!lock_task_sighand(task, &flags)) return ret; if (task_is_traced(task) && !looks_like_a_spurious_pid(task) && !__fatal_signal_pending(task)) { WARN_ON_ONCE(task->jobctl & JOBCTL_DELAY_WAKEKILL); task->jobctl |= JOBCTL_DELAY_WAKEKILL; ret = true; } unlock_task_sighand(task, &flags); return ret; } And nothing there cares about ->__state.
| |