public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Dave Anderson <anderson@redhat.com>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Wang Shu <shuwang@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/3] introduce for_each_process_thread_{break,continue}() helpers
Date: Mon, 25 Jul 2016 18:23:48 +0200	[thread overview]
Message-ID: <20160725162348.GA23947@redhat.com> (raw)
In-Reply-To: <20160725162332.GA23935@redhat.com>

Example:

	rcu_read_lock();
	for_each_process_thread(p, t) {
		do_something_slow(p, t);

		if (SPENT_TOO_MANY_TIME) {
			for_each_process_thread_break(p, t);
			rcu_read_unlock();
			schedule();
			rcu_read_lock();
			for_each_process_thread_continue(&p, &t);
		}
	}
	rcu_read_unlock();

This looks similar to rcu_lock_break(), but much better and the next patch
changes check_hung_uninterruptible_tasks() to use these new helpers. But my
real target is show_state_filter() which can trivially lead to lockup.

Compared to rcu_lock_break(), for_each_process_thread_continue() never gives
up, it relies on fact that both process and thread lists are sorted by the
task->start_time key. So, for example, even if both leader/thread are already
dead we can find the next alive process and continue.

TODO: This is another indication we should rename pid_alive(tsk) (and probably
change it), most users call it to ensure that rcu_read_lock() still protects
this tsk.

NOTE: it seems that, contrary to the comment, task_struct->start_time is not
really monotonic, and this should be probably fixed. Until then  _continue()
might skip more threads with the same ->start_time than necessary.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/sched.h | 10 ++++++++++
 kernel/exit.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 253538f..004c9d5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2859,6 +2859,16 @@ extern bool current_is_single_threaded(void);
 #define for_each_process_thread(p, t)	\
 	for_each_process(p) for_each_thread(p, t)
 
+static inline void
+for_each_process_thread_break(struct task_struct *p, struct task_struct *t)
+{
+	get_task_struct(p);
+	get_task_struct(t);
+}
+
+extern void
+for_each_process_thread_continue(struct task_struct **, struct task_struct **);
+
 static inline int get_nr_threads(struct task_struct *tsk)
 {
 	return tsk->signal->nr_threads;
diff --git a/kernel/exit.c b/kernel/exit.c
index 9e6e135..08f49d4 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -210,6 +210,48 @@ repeat:
 		goto repeat;
 }
 
+void for_each_process_thread_continue(struct task_struct **p_leader,
+				      struct task_struct **p_thread)
+{
+	struct task_struct *leader = *p_leader, *thread = *p_thread;
+	struct task_struct *prev, *next;
+	u64 start_time;
+
+	if (pid_alive(thread)) {
+		/* mt exec could change the leader */
+		*p_leader = thread->group_leader;
+	} else if (pid_alive(leader)) {
+		start_time = thread->start_time;
+		prev = leader;
+
+		for_each_thread(leader, next) {
+			if (next->start_time > start_time)
+				break;
+			prev = next;
+		}
+
+		*p_thread = prev;
+	} else {
+		start_time = leader->start_time;
+		prev = &init_task;
+
+		for_each_process(next) {
+			if (next->start_time > start_time)
+				break;
+			prev = next;
+		}
+
+		*p_leader = prev;
+		/* a new thread can come after that, but this is fine */
+		*p_thread = list_last_entry(&prev->signal->thread_head,
+						struct task_struct,
+						thread_node);
+	}
+
+	put_task_struct(leader);
+	put_task_struct(thread);
+}
+
 /*
  * Determine if a process group is "orphaned", according to the POSIX
  * definition in 2.2.2.52.  Orphaned process groups are not to be affected
-- 
2.5.0

  reply	other threads:[~2016-07-25 16:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-25 16:23 [PATCH 0/3] introduce for_each_process_thread_{break,continue}() helpers Oleg Nesterov
2016-07-25 16:23 ` Oleg Nesterov [this message]
2016-08-02 11:58   ` [PATCH 1/3] " Peter Zijlstra
2016-08-03 20:26     ` Oleg Nesterov
2016-08-08 12:20       ` Peter Zijlstra
2016-07-25 16:23 ` [PATCH 2/3] hung_task.c: change rcu_lock_break() code to use for_each_process_thread_break/continue Oleg Nesterov
2016-07-25 16:23 ` [PATCH 3/3] hung_task.c: change the "max_count" " Oleg Nesterov
2016-07-26 18:57 ` [PATCH 0/1] (Was: introduce for_each_process_thread_{break,continue}() helpers) Oleg Nesterov
2016-07-26 18:57   ` [PATCH 1/1] stop_machine: touch_nmi_watchdog() after MULTI_STOP_PREPARE Oleg Nesterov
2016-07-27  8:06     ` Thomas Gleixner
2016-07-27 10:42     ` [tip:core/urgent] stop_machine: Touch_nmi_watchdog() " tip-bot for Oleg Nesterov

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=20160725162348.GA23947@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=anderson@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=shuwang@redhat.com \
    /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