mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Wander Lairson Costa <wander@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Valentin Schneider <vschneid@redhat.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Oleg Nesterov <oleg@redhat.com>, Brian Cain <bcain@quicinc.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Christian Brauner <brauner@kernel.org>,
	Andrei Vagin <avagin@gmail.com>,
	Shakeel Butt <shakeelb@google.com>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:PERFORMANCE EVENTS SUBSYSTEM" 
	<linux-perf-users@vger.kernel.org>, Hu Chunyu <chuhu@redhat.com>,
	Paul McKenney <paulmck@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v7 2/3] sched/task: Add the put_task_struct_atomic_safe() function
Date: Thu, 4 May 2023 10:42:29 +0200	[thread overview]
Message-ID: <20230504084229.GI1734100@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20230425114307.36889-3-wander@redhat.com>

On Tue, Apr 25, 2023 at 08:43:02AM -0300, Wander Lairson Costa wrote:
> diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
> index b597b97b1f8f..cf774b83b2ec 100644
> --- a/include/linux/sched/task.h
> +++ b/include/linux/sched/task.h
> @@ -141,6 +141,41 @@ static inline void put_task_struct_many(struct task_struct *t, int nr)
>  
>  void put_task_struct_rcu_user(struct task_struct *task);
>  
> +extern void __delayed_put_task_struct(struct rcu_head *rhp);
> +
> +static inline void put_task_struct_atomic_safe(struct task_struct *task)
> +{
> +	if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
> +		/*
> +		 * Decrement the refcount explicitly to avoid unnecessarily
> +		 * calling call_rcu.
> +		 */
> +		if (refcount_dec_and_test(&task->usage))
> +			/*
> +			 * 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()
> +			 * to be called in process context.
> +			 *
> +			 * __put_task_struct() is called when
> +			 * refcount_dec_and_test(&t->usage) succeeds.
> +			 *
> +			 * This means that it can't conflict with
> +			 * put_task_struct_rcu_user() which abuses ->rcu the same
> +			 * way; rcu_users has a reference so task->usage can't be
> +			 * zero after rcu_users 1 -> 0 transition.
> +			 *
> +			 * delayed_free_task() also uses ->rcu, but it is only called
> +			 * when it fails to fork a process. Therefore, there is no
> +			 * way it can conflict with put_task_struct().
> +			 */
> +			call_rcu(&task->rcu, __delayed_put_task_struct);
> +	} else {
> +		put_task_struct(task);
> +	}
> +}

Urgh.. that's plenty horrible. And I'm sure everybody plus kitchen sink
has already asked why can't we just rcu free the thing unconditionally.

Google only found me an earlier version of this same patch set, but I'm
sure we've had that discussion many times over the past several years.
The above and your follow up patch is just horrible.

It requires users to know the RT specific context and gives them no help
what so ever for !RT builds.



  reply	other threads:[~2023-05-04  8:43 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25 11:43 [PATCH v7 0/3] Introduce put_task_struct_atomic_sleep() Wander Lairson Costa
2023-04-25 11:43 ` [PATCH v7 1/3] sched/core: warn on call put_task_struct in invalid context Wander Lairson Costa
2023-04-28 16:17   ` Sebastian Andrzej Siewior
2023-05-02 14:46     ` Wander Lairson Costa
2023-04-25 11:43 ` [PATCH v7 2/3] sched/task: Add the put_task_struct_atomic_safe() function Wander Lairson Costa
2023-05-04  8:42   ` Peter Zijlstra [this message]
2023-05-04  9:32     ` Valentin Schneider
2023-05-04 12:24       ` Wander Lairson Costa
2023-05-04 12:24     ` Wander Lairson Costa
2023-05-04 12:29     ` Oleg Nesterov
2023-05-04 14:33       ` Peter Zijlstra
2023-05-04 14:55         ` Wander Lairson Costa
2023-05-04 15:23           ` Oleg Nesterov
2023-05-04 15:30             ` Peter Zijlstra
2023-05-05 13:39               ` Peter Zijlstra
2023-05-04 18:29             ` Wander Lairson Costa
2023-05-04 19:22               ` Oleg Nesterov
2023-05-04 19:38                 ` Wander Lairson Costa
2023-05-04 20:16                   ` Oleg Nesterov
2023-05-08 12:30                     ` Wander Lairson Costa
2023-05-04 15:24           ` Peter Zijlstra
2023-05-04 18:21             ` Wander Lairson Costa
2023-05-05 13:32               ` Peter Zijlstra
2023-05-05 14:26                 ` Steven Rostedt
2023-05-05 14:29                   ` Steven Rostedt
2023-05-08 12:28                 ` Wander Lairson Costa
2023-04-25 11:43 ` [PATCH v7 3/3] treewide: replace put_task_struct() with the atomic safe version Wander Lairson Costa
2023-04-26 12:05 ` [PATCH v7 0/3] Introduce put_task_struct_atomic_sleep() Valentin Schneider
2023-04-26 17:44 ` Waiman Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230504084229.GI1734100@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=avagin@gmail.com \
    --cc=bcain@quicinc.com \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=chuhu@redhat.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=ebiederm@xmission.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mark.rutland@arm.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=shakeelb@google.com \
    --cc=tglx@linutronix.de \
    --cc=vbabka@suse.cz \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=wander@redhat.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome