insert_work() always tried to wake up a worker; however, the only time it needs to try to wake up a worker is when a new active work item is queued. When a work item goes on the inactive list or queueing a flush work item, there's no reason to try to wake up a worker. This patch moves the worker wakeup logic out of insert_work() and places it in the active new work item queueing path in __queue_work(). While at it: * __queue_work() is dereferencing pwq->pool repeatedly. Add local variable pool. * Every caller of insert_work() calls debug_work_activate(). Consolidate the invocations into insert_work(). * In __queue_work() pool->watchdog_ts update is relocated slightly. This is to better accommodate future changes. This makes wakeups more precise and will help the planned change to assign work items to workers before waking them up. No behavior changes intended. v2: WARN_ON_ONCE(pool != last_pool) added in __queue_work() to clarify as suggested by Lai.
Showing the changes to kernel/workqueue.c (1 of 1 file). Show all
Unified · Split
| @@ -1542,7 +1542,7 @@ fail: · 1541 unmodified lines | ||
| 1542 | 1542 | static void insert_work(struct pool_workqueue *pwq, struct work_struct *work, |
| 1543 | 1543 | struct list_head *head, unsigned int extra_flags) |
| 1544 | 1544 | { |
| 1545 | − struct worker_pool *pool = pwq->pool; | |
| 1545 | + debug_work_activate(work); | |
| 1546 | 1546 | |
| 1547 | 1547 | /* record the work call stack in order to print it in KASAN reports */ |
| 1548 | 1548 | kasan_record_aux_stack_noalloc(work); |
| @@ -1551,9 +1551,6 @@ static void insert_work(struct pool_workqueue *pwq, struct work_struct *work, · 2 unmodified lines | ||
| 1551 | 1551 | set_work_pwq(work, pwq, extra_flags); |
| 1552 | 1552 | list_add_tail(&work->entry, head); |
| 1553 | 1553 | get_pwq(pwq); |
| 1554 | − | |
| 1555 | − if (__need_more_worker(pool)) | |
| 1556 | − wake_up_worker(pool); | |
| 1557 | 1554 | } |
| 1558 | 1555 | |
| 1559 | 1556 | /* |
| @@ -1607,8 +1604,7 @@ static void __queue_work(int cpu, struct workqueue_struct *wq, · 47 unmodified lines | ||
| 1607 | 1604 | struct work_struct *work) |
| 1608 | 1605 | { |
| 1609 | 1606 | struct pool_workqueue *pwq; |
| 1610 | − struct worker_pool *last_pool; | |
| 1611 | − struct list_head *worklist; | |
| 1607 | + struct worker_pool *last_pool, *pool; | |
| 1612 | 1608 | unsigned int work_flags; |
| 1613 | 1609 | unsigned int req_cpu = cpu; |
| 1614 | 1610 | |
| @@ -1642,13 +1638,15 @@ retry: · 27 unmodified lines | ||
| 1642 | 1638 | pwq = per_cpu_ptr(wq->cpu_pwqs, cpu); |
| 1643 | 1639 | } |
| 1644 | 1640 | |
| 1641 | + pool = pwq->pool; | |
| 1642 | + | |
| 1645 | 1643 | /* |
| 1646 | 1644 | * If @work was previously on a different pool, it might still be |
| 1647 | 1645 | * running there, in which case the work needs to be queued on that |
| 1648 | 1646 | * pool to guarantee non-reentrancy. |
| 1649 | 1647 | */ |
| 1650 | 1648 | last_pool = get_work_pool(work); |
| 1651 | − if (last_pool && last_pool != pwq->pool) { | |
| 1649 | + if (last_pool && last_pool != pool) { | |
| 1652 | 1650 | struct worker *worker; |
| 1653 | 1651 | |
| 1654 | 1652 | raw_spin_lock(&last_pool->lock); |
| @@ -1657,13 +1655,15 @@ retry: · 2 unmodified lines | ||
| 1657 | 1655 | |
| 1658 | 1656 | if (worker && worker->current_pwq->wq == wq) { |
| 1659 | 1657 | pwq = worker->current_pwq; |
| 1658 | + pool = pwq->pool; | |
| 1659 | + WARN_ON_ONCE(pool != last_pool); | |
| 1660 | 1660 | } else { |
| 1661 | 1661 | /* meh... not running there, queue here */ |
| 1662 | 1662 | raw_spin_unlock(&last_pool->lock); |
| 1663 | − raw_spin_lock(&pwq->pool->lock); | |
| 1663 | + raw_spin_lock(&pool->lock); | |
| 1664 | 1664 | } |
| 1665 | 1665 | } else { |
| 1666 | − raw_spin_lock(&pwq->pool->lock); | |
| 1666 | + raw_spin_lock(&pool->lock); | |
| 1667 | 1667 | } |
| 1668 | 1668 | |
| 1669 | 1669 | /* |
| @@ -1676,7 +1676,7 @@ retry: · 6 unmodified lines | ||
| 1676 | 1676 | */ |
| 1677 | 1677 | if (unlikely(!pwq->refcnt)) { |
| 1678 | 1678 | if (wq->flags & WQ_UNBOUND) { |
| 1679 | − raw_spin_unlock(&pwq->pool->lock); | |
| 1679 | + raw_spin_unlock(&pool->lock); | |
| 1680 | 1680 | cpu_relax(); |
| 1681 | 1681 | goto retry; |
| 1682 | 1682 | } |
| @@ -1695,21 +1695,22 @@ retry: · 12 unmodified lines | ||
| 1695 | 1695 | work_flags = work_color_to_flags(pwq->work_color); |
| 1696 | 1696 | |
| 1697 | 1697 | if (likely(pwq->nr_active < pwq->max_active)) { |
| 1698 | + if (list_empty(&pool->worklist)) | |
| 1699 | + pool->watchdog_ts = jiffies; | |
| 1700 | + | |
| 1698 | 1701 | trace_workqueue_activate_work(work); |
| 1699 | 1702 | pwq->nr_active++; |
| 1700 | − worklist = &pwq->pool->worklist; | |
| 1701 | − if (list_empty(worklist)) | |
| 1702 | − pwq->pool->watchdog_ts = jiffies; | |
| 1703 | + insert_work(pwq, work, &pool->worklist, work_flags); | |
| 1704 | + | |
| 1705 | + if (__need_more_worker(pool)) | |
| 1706 | + wake_up_worker(pool); | |
| 1703 | 1707 | } else { |
| 1704 | 1708 | work_flags |= WORK_STRUCT_INACTIVE; |
| 1705 | − worklist = &pwq->inactive_works; | |
| 1709 | + insert_work(pwq, work, &pwq->inactive_works, work_flags); | |
| 1706 | 1710 | } |
| 1707 | 1711 | |
| 1708 | − debug_work_activate(work); | |
| 1709 | − insert_work(pwq, work, worklist, work_flags); | |
| 1710 | − | |
| 1711 | 1712 | out: |
| 1712 | − raw_spin_unlock(&pwq->pool->lock); | |
| 1713 | + raw_spin_unlock(&pool->lock); | |
| 1713 | 1714 | rcu_read_unlock(); |
| 1714 | 1715 | } |
| 1715 | 1716 | |
| @@ -3012,7 +3013,6 @@ static void insert_wq_barrier(struct pool_workqueue *pwq, · 1296 unmodified lines | ||
| 3012 | 3013 | pwq->nr_in_flight[work_color]++; |
| 3013 | 3014 | work_flags |= work_color_to_flags(work_color); |
| 3014 | 3015 | |
| 3015 | − debug_work_activate(&barr->work); | |
| 3016 | 3016 | insert_work(pwq, &barr->work, head, work_flags); |
| 3017 | 3017 | } |
| 3018 | 3018 | |