linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>, Al Viro <viro@zeniv.linux.org.uk>
Cc: Dave Jones <davej@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	rostedt <rostedt@goodmis.org>, dhowells <dhowells@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 2/4] task_work: task_work_add() should not succeed after exit_task_work()
Date: Sun, 26 Aug 2012 21:12:11 +0200	[thread overview]
Message-ID: <20120826191211.GA4228@redhat.com> (raw)
In-Reply-To: <20120826191132.GA3743@redhat.com>

ed3e694d "move exit_task_work() past exit_files() et.al" destroyed
the add/exit synchronization we had, the caller itself should ensure
task_work_add() can't race with the exiting task.

However, this is not convenient/simple, and the only user which tries
to do this is buggy (see the next patch). Unless the task is current,
there is simply no way to do this in general.

Change exit_task_work()->task_work_run() to use the dummy "work_exited"
entry to let task_work_add() know it should fail.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/task_work.h |    3 +--
 kernel/task_work.c        |   22 ++++++++++++++++------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/linux/task_work.h b/include/linux/task_work.h
index fb46b03..ca5a1cf 100644
--- a/include/linux/task_work.h
+++ b/include/linux/task_work.h
@@ -18,8 +18,7 @@ void task_work_run(void);
 
 static inline void exit_task_work(struct task_struct *task)
 {
-	if (unlikely(task->task_works))
-		task_work_run();
+	task_work_run();
 }
 
 #endif	/* _LINUX_TASK_WORK_H */
diff --git a/kernel/task_work.c b/kernel/task_work.c
index f13ec0b..65bd3c9 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -2,16 +2,17 @@
 #include <linux/task_work.h>
 #include <linux/tracehook.h>
 
+static struct callback_head work_exited; /* all we need is ->next == NULL */
+
 int
 task_work_add(struct task_struct *task, struct callback_head *work, bool notify)
 {
 	struct callback_head *head;
-	/*
-	 * Not inserting the new work if the task has already passed
-	 * exit_task_work() is the responisbility of callers.
-	 */
+
 	do {
 		head = ACCESS_ONCE(task->task_works);
+		if (unlikely(head == &work_exited))
+			return -ESRCH;
 		work->next = head;
 	} while (cmpxchg(&task->task_works, head, work) != head);
 
@@ -30,7 +31,7 @@ task_work_cancel(struct task_struct *task, task_work_func_t func)
 	 * If cmpxchg() fails we continue without updating pprev.
 	 * Either we raced with task_work_add() which added the
 	 * new entry before this work, we will find it again. Or
-	 * we raced with task_work_run(), *pprev == NULL.
+	 * we raced with task_work_run(), *pprev == NULL/exited.
 	 */
 	raw_spin_lock_irqsave(&task->pi_lock, flags);
 	while ((work = ACCESS_ONCE(*pprev))) {
@@ -51,7 +52,16 @@ void task_work_run(void)
 	struct callback_head *work, *head, *next;
 
 	for (;;) {
-		work = xchg(&task->task_works, NULL);
+		/*
+		 * work->func() can do task_work_add(), do not set
+		 * work_exited unless the list is empty.
+		 */
+		do {
+			work = ACCESS_ONCE(task->task_works);
+			head = !work && (task->flags & PF_EXITING) ?
+				&work_exited : NULL;
+		} while (cmpxchg(&task->task_works, work, head) != work);
+
 		if (!work)
 			break;
 		/*
-- 
1.5.5.1


  parent reply	other threads:[~2012-08-26 19:10 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-24 20:36 lockdep trace from posix timers Dave Jones
2012-07-27 16:20 ` Dave Jones
2012-08-16 12:54   ` Ming Lei
2012-08-16 14:03     ` Dave Jones
2012-08-16 18:07 ` Peter Zijlstra
2012-08-17 15:14   ` Oleg Nesterov
2012-08-17 15:17     ` Oleg Nesterov
2012-08-17 16:40       ` task_work_add() should not succeed unconditionally (Was: lockdep trace from posix timers) Oleg Nesterov
2012-08-20  7:15     ` lockdep trace from posix timers Peter Zijlstra
2012-08-20 11:44       ` Peter Zijlstra
2012-08-20 11:46         ` Peter Zijlstra
2012-08-20 11:50         ` Peter Zijlstra
2012-08-20 12:19           ` Steven Rostedt
2012-08-20 12:20             ` Peter Zijlstra
2012-08-20 14:59         ` Oleg Nesterov
2012-08-20 15:10           ` Peter Zijlstra
2012-08-20 15:27             ` Peter Zijlstra
2012-08-20 15:32               ` Oleg Nesterov
2012-08-20 15:46                 ` Peter Zijlstra
2012-08-20 15:58                   ` Oleg Nesterov
2012-08-20 16:03                     ` Peter Zijlstra
2012-08-20 15:05         ` Oleg Nesterov
2012-08-20 15:12           ` Peter Zijlstra
2012-08-20 15:41             ` Oleg Nesterov
2012-08-20 15:56               ` Peter Zijlstra
2012-08-20 16:10                 ` Oleg Nesterov
2012-08-20 16:19                   ` Peter Zijlstra
2012-08-20 16:23                     ` Oleg Nesterov
2012-08-21 18:27                       ` Oleg Nesterov
2012-08-21 18:34                         ` Oleg Nesterov
2012-08-24 18:56                           ` Oleg Nesterov
2012-08-26 19:11                             ` [PATCH 0/4] (Was: lockdep trace from posix timers) Oleg Nesterov
2012-08-26 19:12                               ` [PATCH 1/4] task_work: make task_work_add() lockless Oleg Nesterov
2012-09-14  6:08                                 ` [tip:core/urgent] task_work: Make " tip-bot for Oleg Nesterov
2012-09-24 19:27                                 ` [PATCH 1/4] task_work: make " Geert Uytterhoeven
2012-09-24 20:37                                   ` Oleg Nesterov
2012-08-26 19:12                               ` Oleg Nesterov [this message]
2012-09-14  6:09                                 ` [tip:core/urgent] task_work: task_work_add() should not succeed after exit_task_work() tip-bot for Oleg Nesterov
2012-08-26 19:12                               ` [PATCH 3/4] task_work: revert d35abdb2 "hold task_lock around checks in keyctl" Oleg Nesterov
2012-09-14  6:10                                 ` [tip:core/urgent] task_work: Revert " hold " tip-bot for Oleg Nesterov
2012-08-26 19:12                               ` [PATCH 4/4] task_work: simplify the usage in ptrace_notify() and get_signal_to_deliver() Oleg Nesterov
2012-09-14  6:11                                 ` [tip:core/urgent] task_work: Simplify " tip-bot for Oleg Nesterov
2012-09-06 18:01                               ` [PATCH 0/4] (Was: lockdep trace from posix timers) Oleg Nesterov
2012-09-06 18:35                                 ` Peter Zijlstra
2012-09-07 13:13                                   ` Oleg Nesterov
2012-08-28 16:29                             ` lockdep trace from posix timers Peter Zijlstra
2012-08-28 17:01                               ` Oleg Nesterov
2012-08-28 17:12                                 ` Oleg Nesterov
2012-08-28 17:28                                 ` Peter Zijlstra
2012-08-29 15:25                                   ` Oleg Nesterov
2012-08-20 14:55       ` Oleg Nesterov
2012-08-20 15:43       ` Oleg Nesterov
2012-08-20 15:48         ` Peter Zijlstra
2012-08-20 15:58           ` 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=20120826191211.GA4228@redhat.com \
    --to=oleg@redhat.com \
    --cc=davej@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).