Messages in this thread | | | From | Wander Lairson Costa <> | Date | Thu, 2 Feb 2023 16:55:34 -0300 | Subject | Re: [PATCH v3] kernel/fork: beware of __put_task_struct calling context |
| |
On Thu, Feb 2, 2023 at 3:37 PM, Oleg Nesterov <oleg@redhat.com> wrote: > > On 02/01, Wander Lairson Costa wrote: > > > > Instead of calling __put_task_struct() directly, we defer it using > > call_rcu(). A more natural approach would use a workqueue, but since > > in PREEMPT_RT, we can't allocate dynamic memory from atomic context, > > the code would become more complex because we would need to put the > > work_struct instance in the task_struct and initialize it when we > > allocate a new task_struct. > > I don't think I can ack the changes in PREEMPT_RT but this version LGTM. > > > > > just a couple of purely cosmetic nits, feel free to ignore... > > > +static void __delayed_put_task_struct(struct rcu_head *rhp) > > +{ > > + struct task_struct *task = container_of(rhp, struct task_struct, rcu); > > + > > + ___put_task_struct(task); > > +} > > We already have delayed_put_task_struct() which differs very much. > Perhaps something like ___put_task_struct() will look less confusing. >
___put_task_struct()? I already added a function with this name below.
> > +void __put_task_struct(struct task_struct *tsk) > > +{ > > + if (IS_ENABLED(CONFIG_PREEMPT_RT) && (!preemptible() || !in_task())) > > + /* > > + * under PREEMPT_RT, we can't call put_task_struct > > + * in atomic context because it will indirectly > > + * acquire sleeping locks. > > + */ > > + call_rcu(&tsk->rcu, __delayed_put_task_struct); > > Perhaps this deserves additional note to explain why is it safe to use tsk->rcu > union. May be this is obvious, but I was confused when I looked at the previous > version ;) >
Makes sense, I will add it in the next version.
> but again, feel free to ignore. > > Oleg. >
| |