lkml.org 
[lkml]   [2022]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2022-04-21 22:08    [W:0.169 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site