task_work_run: don't take ->pi_lock unconditionally

As Peter pointed out, task_work() can avoid ->pi_lock and cmpxchg()
if task->task_works == NULL && !PF_EXITING.

And in fact the only reason why task_work_run() needs ->pi_lock is
the possible race with task_work_cancel(), we can optimize this code
and make the locking more clear.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/kernel/task_work.c b/kernel/task_work.c
index 0fef395662a6ea6f38301e92f74d14f60b3f145d..825f28259a19a285399171df9007ab01c19b42b1 100644 (file)
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -97,16 +97,26 @@ void task_work_run(void)
                 * work->func() can do task_work_add(), do not set
                 * work_exited unless the list is empty.
                 */
-               raw_spin_lock_irq(&task->pi_lock);
                do {
+                       head = NULL;
                        work = READ_ONCE(task->task_works);
-                       head = !work && (task->flags & PF_EXITING) ?
-                               &work_exited : NULL;
+                       if (!work) {
+                               if (task->flags & PF_EXITING)
+                                       head = &work_exited;
+                               else
+                                       break;
+                       }
                } while (cmpxchg(&task->task_works, work, head) != work);
-               raw_spin_unlock_irq(&task->pi_lock);
 
                if (!work)
                        break;
+               /*
+                * Synchronize with task_work_cancel(). It can not remove
+                * the first entry == work, cmpxchg(task_works) must fail.
+                * But it can remove another entry from the ->next list.
+                */
+               raw_spin_lock_irq(&task->pi_lock);
+               raw_spin_unlock_irq(&task->pi_lock);
 
                do {
                        next = work->next;