If JOBCTL_TASK_WORK is already set on the targeted task, then we need not go through {lock,unlock}_task_sighand() to set it again and queue a signal wakeup. This is safe as we're checking it _after_ adding the new task_work with cmpxchg(). The ordering is as follows: task_work_add() get_signal() -------------------------------------------------------------- STORE(task->task_works, new_work); STORE(task->jobctl); mb(); mb(); LOAD(task->jobctl); LOAD(task->task_works); This speeds up TWA_SIGNAL handling quite a bit, which is important now that io_uring is relying on it for all task_work deliveries.
Showing the changes to kernel (2 of 2 files). Show all
Unified · Split
| @@ -2541,7 +2541,21 @@ bool get_signal(struct ksignal *ksig) · 2540 unmodified lines | ||
| 2541 | 2541 | |
| 2542 | 2542 | relock: |
| 2543 | 2543 | spin_lock_irq(&sighand->siglock); |
| 2544 | − current->jobctl &= ~JOBCTL_TASK_WORK; | |
| 2544 | + /* | |
| 2545 | + * Make sure we can safely read ->jobctl() in task_work add. As Oleg | |
| 2546 | + * states: | |
| 2547 | + * | |
| 2548 | + * It pairs with mb (implied by cmpxchg) before READ_ONCE. So we | |
| 2549 | + * roughly have | |
| 2550 | + * | |
| 2551 | + * task_work_add: get_signal: | |
| 2552 | + * STORE(task->task_works, new_work); STORE(task->jobctl); | |
| 2553 | + * mb(); mb(); | |
| 2554 | + * LOAD(task->jobctl); LOAD(task->task_works); | |
| 2555 | + * | |
| 2556 | + * and we can rely on STORE-MB-LOAD [ in task_work_add]. | |
| 2557 | + */ | |
| 2558 | + smp_store_mb(current->jobctl, current->jobctl & ~JOBCTL_TASK_WORK); | |
| 2545 | 2559 | if (unlikely(current->task_works)) { |
| 2546 | 2560 | spin_unlock_irq(&sighand->siglock); |
| 2547 | 2561 | task_work_run(); |
| @@ -42,7 +42,13 @@ task_work_add(struct task_struct *task, struct callback_head *work, int notify) · 41 unmodified lines | ||
| 42 | 42 | set_notify_resume(task); |
| 43 | 43 | break; |
| 44 | 44 | case TWA_SIGNAL: |
| 45 | − if (lock_task_sighand(task, &flags)) { | |
| 45 | + /* | |
| 46 | + * Only grab the sighand lock if we don't already have some | |
| 47 | + * task_work pending. This pairs with the smp_store_mb() | |
| 48 | + * in get_signal(), see comment there. | |
| 49 | + */ | |
| 50 | + if (!(READ_ONCE(task->jobctl) & JOBCTL_TASK_WORK) && | |
| 51 | + lock_task_sighand(task, &flags)) { | |
| 46 | 52 | task->jobctl |= JOBCTL_TASK_WORK; |
| 47 | 53 | signal_wake_up(task, 0); |
| 48 | 54 | unlock_task_sighand(task, &flags); |