Messages in this thread | | | From | Joel Fernandes <> | Date | Mon, 10 May 2021 12:06:38 -0400 | Subject | Re: [PATCH 17/19] sched: Inherit task cookie on fork() |
| |
Hi Peter,
On Thu, Apr 22, 2021 at 8:36 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Note that sched_core_fork() is called from under tasklist_lock, and > not from sched_fork() earlier. This avoids a few races later. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > include/linux/sched.h | 2 ++ > kernel/fork.c | 3 +++ > kernel/sched/core_sched.c | 6 ++++++ > 3 files changed, 11 insertions(+) > > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -2172,8 +2172,10 @@ const struct cpumask *sched_trace_rd_spa > > #ifdef CONFIG_SCHED_CORE > extern void sched_core_free(struct task_struct *tsk); > +extern void sched_core_fork(struct task_struct *p); > #else > static inline void sched_core_free(struct task_struct *tsk) { } > +static inline void sched_core_fork(struct task_struct *p) { } > #endif > > #endif > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -2249,6 +2249,8 @@ static __latent_entropy struct task_stru > > klp_copy_process(p); > > + sched_core_fork(p); > + > spin_lock(¤t->sighand->siglock); > > /* > @@ -2336,6 +2338,7 @@ static __latent_entropy struct task_stru > return p; > > bad_fork_cancel_cgroup: > + sched_core_free(p); > spin_unlock(¤t->sighand->siglock); > write_unlock_irq(&tasklist_lock); > cgroup_cancel_fork(p, args); > --- a/kernel/sched/core_sched.c > +++ b/kernel/sched/core_sched.c > @@ -100,6 +100,12 @@ static unsigned long sched_core_clone_co > return cookie; > } > > +void sched_core_fork(struct task_struct *p) > +{ > + RB_CLEAR_NODE(&p->core_node); > + p->core_cookie = sched_core_clone_cookie(current);
Does this make sense also for !CLONE_THREAD forks?
With earlier versions of core scheduling, we have done the following on ChromeOS. Basically, if it is a "thread clone", share the cookie since memory is shared within a process (same address space within a process). Otherwise, set the cookie to a new unique cookie so that the new process does not share core with parent initially (since their address space will be different).
Example Psedu-ocode in sched_fork():
if (current->core_cookie && (clone_flags & CLONE_THREAD)) { p->core_cookie = clone_cookie(current); } else { p->core_cookie = create_new_cookie(); }
In your version though, I don't see that it always clones the cookie whether it is a CLONE_THREAD clone or not. Is that correct? I feel that's a security issue.
-Joel
| |