Re: [PATCH v3] kernel/fork: beware of __put_task_struct calling context

[Oleg Nesterov <oleg@redhat.com>]

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.

> +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 ;)

but again, feel free to ignore.

Oleg.