On 10/09/2012 07:04 PM, Peter Zijlstra wrote:
> On Mon, 2012-10-08 at 14:38 +0200, Oleg Nesterov wrote:
>> But the code looks more complex, and the only advantage is that
>> non-exiting task does xchg() instead of cmpxchg(). Not sure this
>> worth the trouble, in this case task_work_run() will likey run
>> the callbacks (the caller checks ->task_works != NULL), I do not
>> think this can add any noticeable speedup.
>
> Yeah, I agree, the patch doesn't seem worth the trouble. It makes tricky
> code unreadable at best.
>
To gain better readability, we need to move work_exited things out
from task_work_run() too.
Thanks,
Lai
Subject: task_work: avoid unneeded cmpxchg() in task_work_run()
We only require cmpxchg()&retry when task is exiting.
xchg() is enough in other cases like original code in ac3d0da8.
So we use xchg() for task_work_run() and move the logic
of exit_task_work() out from task_work_run().
Signed-off-by: Lai Jiangshan <la...@cn.fujitsu.com>
---
diff --git a/include/linux/task_work.h b/include/linux/task_work.h
index ca5a1cf..1e686a5 100644
--- a/include/linux/task_work.h
+++ b/include/linux/task_work.h
@@ -15,10 +15,6 @@ init_task_work(struct callback_head *twork, task_work_func_t
func)
int task_work_add(struct task_struct *task, struct callback_head *twork, bool);
struct callback_head *task_work_cancel(struct task_struct *, task_work_func_t);
void task_work_run(void);
-
-static inline void exit_task_work(struct task_struct *task)
-{
- task_work_run();
-}
+void exit_task_work(struct task_struct *task);
#endif /* _LINUX_TASK_WORK_H */
diff --git a/kernel/task_work.c b/kernel/task_work.c
index 65bd3c9..87ef3b7 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -52,16 +52,7 @@ void task_work_run(void)
struct callback_head *work, *head, *next;
for (;;) {
- /*
- * 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);
-
+ work = xchg(&task->task_works, NULL);
if (!work)
break;
/*
@@ -90,3 +81,17 @@ void task_work_run(void)
} while (work);
}
}
+
+void exit_task_work(struct task_struct *task)
+{
+ for (;;) {
+ /*
+ * work->func() can do task_work_add(), do not set
+ * work_exited unless the list is empty.
+ */
+ if (unlikely(task->task_works))
+ task_work_run();
+ if (cmpxchg(&task->task_works, NULL, &work_exited) == NULL)
+ break;
+ }
+}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
