Skip to content

Commit

Permalink
task_work: task_work_add() should not succeed after exit_task_work()
Browse files Browse the repository at this point in the history
ed3e694 "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 <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
  • Loading branch information
oleg-nesterov authored and Ingo Molnar committed Sep 13, 2012
1 parent ac3d0da commit 9da33de
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
3 changes: 1 addition & 2 deletions include/linux/task_work.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
22 changes: 16 additions & 6 deletions kernel/task_work.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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))) {
Expand All @@ -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;
/*
Expand Down

0 comments on commit 9da33de

Please sign in to comment.