Messages in this thread Patch in this message | | | Date | Thu, 5 Nov 2020 10:46:53 -0500 | From | Joel Fernandes <> | Subject | Re: [PATCH v8 -tip 20/26] sched: Release references to the per-task cookie on exit |
| |
On Wed, Nov 04, 2020 at 04:50:42PM -0500, chris hyser wrote: > On 10/19/20 9:43 PM, Joel Fernandes (Google) wrote: > > During exit, we have to free the references to a cookie that might be shared by > > many tasks. This commit therefore ensures when the task_struct is released, any > > references to cookies that it holds are also released. > > > > Tested-by: Julien Desfossez <jdesfossez@digitalocean.com> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > --- > > include/linux/sched.h | 2 ++ > > kernel/fork.c | 1 + > > kernel/sched/core.c | 8 ++++++++ > > 3 files changed, 11 insertions(+) > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > index 4cb76575afa8..eabd96beff92 100644 > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -2079,12 +2079,14 @@ void sched_core_unsafe_exit(void); > > bool sched_core_wait_till_safe(unsigned long ti_check); > > bool sched_core_kernel_protected(void); > > int sched_core_share_pid(pid_t pid); > > +void sched_tsk_free(struct task_struct *tsk); > > #else > > #define sched_core_unsafe_enter(ignore) do { } while (0) > > #define sched_core_unsafe_exit(ignore) do { } while (0) > > #define sched_core_wait_till_safe(ignore) do { } while (0) > > #define sched_core_kernel_protected(ignore) do { } while (0) > > #define sched_core_share_pid(pid_t pid) do { } while (0) > > +#define sched_tsk_free(tsk) do { } while (0) > > #endif > > #endif > > diff --git a/kernel/fork.c b/kernel/fork.c > > index b9c289d0f4ef..a39248a02fdd 100644 > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -736,6 +736,7 @@ void __put_task_struct(struct task_struct *tsk) > > exit_creds(tsk); > > delayacct_tsk_free(tsk); > > put_signal_struct(tsk->signal); > > + sched_tsk_free(tsk); > > if (!profile_handoff_task(tsk)) > > free_task(tsk); > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index 42aa811eab14..61e1dcf11000 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -9631,6 +9631,14 @@ static int cpu_core_tag_color_write_u64(struct cgroup_subsys_state *css, > > return 0; > > } > > + > > +void sched_tsk_free(struct task_struct *tsk) > > +{ > > + if (!tsk->core_task_cookie) > > + return; > > + sched_core_put_task_cookie(tsk->core_task_cookie); > > + sched_core_put(); > > > sched_tsk_free() can be called under softirq. sched_core_put() is riddled with things that may want to sleep.
Right, that breaks. Can you try the diff I attached below and see if it does not crash now?
> I tried a number of things like replacing the mutexes in put/get with spin_lock_bh() and just kept finding things like: > > > [root@chyser-vm5 bin]# [ 1123.516209] BUG: sleeping function called from invalid context at include/linux/percpu-rwsem.h:49
Indeed, stop machine cannot be called from an atomic context, which is what spin_lock_bh gives you.
---8<-----------------------
diff --git a/kernel/sched/coretag.c b/kernel/sched/coretag.c index 6c008e5471d7..9967f37c5df0 100644 --- a/kernel/sched/coretag.c +++ b/kernel/sched/coretag.c @@ -16,6 +16,7 @@ */ struct sched_core_cookie { refcount_t refcnt; + struct work_struct work; /* to free in WQ context. */; }; static DEFINE_MUTEX(sched_core_tasks_mutex); @@ -54,21 +55,24 @@ void sched_core_tag_requeue(struct task_struct *p, unsigned long cookie, bool gr } /* Per-task interface: Used by fork(2) and prctl(2). */ +static void sched_core_put_cookie_work(struct work_struct *ws); + static unsigned long sched_core_alloc_task_cookie(void) { - struct sched_core_cookie *ptr = + struct sched_core_cookie *ck = kmalloc(sizeof(struct sched_core_cookie), GFP_KERNEL); - if (!ptr) + if (!ck) return 0; - refcount_set(&ptr->refcnt, 1); + refcount_set(&ck->refcnt, 1); + INIT_WORK(&ck->work, sched_core_put_cookie_work); /* * NOTE: sched_core_put() is not done by put_task_cookie(). Instead, it * is done after the stopper runs. */ sched_core_get(); - return (unsigned long)ptr; + return (unsigned long)ck; } static bool sched_core_get_task_cookie(unsigned long cookie) @@ -91,6 +95,15 @@ static void sched_core_put_task_cookie(unsigned long cookie) kfree(ptr); } +static void sched_core_put_cookie_work(struct work_struct *ws) +{ + struct sched_core_cookie *ck = + container_of(ws, struct sched_core_cookie, work); + + sched_core_put_task_cookie((unsigned long)ck); + sched_core_put(); +} + struct sched_core_task_write_tag { struct task_struct *tasks[2]; unsigned long cookies[2]; @@ -461,8 +474,11 @@ int cpu_core_tag_color_write_u64(struct cgroup_subsys_state *css, void sched_tsk_free(struct task_struct *tsk) { + struct sched_core_cookie *ck; + if (!tsk->core_task_cookie) return; - sched_core_put_task_cookie(tsk->core_task_cookie); - sched_core_put(); + + ck = (struct sched_core_cookie *)tsk->core_task_cookie; + queue_work(system_wq, &ck->work); }
| |