Messages in this thread | | | From | Wander Lairson Costa <> | Date | Mon, 6 Feb 2023 15:32:54 -0300 | Subject | Re: [PATCH v4] kernel/fork: beware of __put_task_struct calling context |
| |
On Mon, Feb 6, 2023 at 11:57 AM Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > On 2023-02-06 10:04:47 [-0300], Wander Lairson Costa wrote: > > Under PREEMPT_RT, __put_task_struct() indirectly acquires sleeping > > locks. Therefore, it can't be called from an non-preemptible context. > > > > One practical example is splat inside inactive_task_timer(), which is > > called in a interrupt context: > > Do you have more? > The inactive_task_timer() is marked as HRTIMER_MODE_REL_HARD which means > in runs in hardirq-context. The author of commit > 850377a875a48 ("sched/deadline: Ensure inactive_timer runs in hardirq context") > > should have been aware of that. > We have on workaround of that put_task() in sched-switch. I wasn't aware > of this shortcoming. So either we have more problems or potential > problems or this is the only finding so far. >
Valentin spotted two other potential issues, I fixed them in v2[1]. Also there is a discussion there that led to this implementation.
> > diff --git a/kernel/fork.c b/kernel/fork.c > > index 9f7fe3541897..532dd2ceb6a3 100644 > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -857,6 +857,29 @@ void __put_task_struct(struct task_struct *tsk) > … > > +void __put_task_struct(struct task_struct *tsk) > > +{ > > + if (IS_ENABLED(CONFIG_PREEMPT_RT) && (!preemptible() || !in_task())) > > Is it safe to use the rcu member in any case? If so why not use it > unconditionally? >
I am unsure what would be the consequences of moving every call to RCU, so I thought it would be better to play safe and do it only when necessary.
> > + /* > > + * under PREEMPT_RT, we can't call put_task_struct > > + * in atomic context because it will indirectly > > + * acquire sleeping locks. > > + * > > + * call_rcu() will schedule delayed_put_task_struct_rcu() > > + * to be called in process context. > > + */ > > + call_rcu(&tsk->rcu, delayed_put_task_struct_rcu); > > + else > > + ___put_task_struct(tsk); > > +} > > EXPORT_SYMBOL_GPL(__put_task_struct); > > > > void __init __weak arch_task_cache_init(void) { } > > Sebastian >
[1] https://lore.kernel.org/all/20230120150246.20797-1-wander@redhat.com/
| |