lkml.org 
[lkml]   [2023]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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/

\
 
 \ /
  Last update: 2023-03-27 00:11    [W:0.234 / U:0.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site