Re: [PATCH v4 4/4] workqueue: Unbind workers before sending them to exit()

Hillf Danton
2022-10-05T14:50:42Z

On 05 Oct 2022 12:13:17 +0100 Valentin Schneider <vschneid@redhat.com>
>On 05/10/22 09:08, Hillf Danton wrote:
>> On 4 Oct 2022 16:05:21 +0100 Valentin Schneider <vschneid@redhat.com>
>>> It has been reported that isolated CPUs can suffer from interference due to
>>> per-CPU kworkers waking up just to die.
>>>
>>> A surge of workqueue activity during initial setup of a latency-sensitive
>>> application (refresh_vm_stats() being one of the culprits) can cause extra
>>> per-CPU kworkers to be spawned. Then, said latency-sensitive task can be
>>> running merrily on an isolated CPU only to be interrupted sometime later by
>>> a kworker marked for death (cf. IDLE_WORKER_TIMEOUT, 5 minutes after last
>>> kworker activity).
>>>
>> Is tick stopped on the isolated CPU? If tick can hit it then it can accept
>> more than exiting kworker.
>
>From what I've seen in the scenarios where that happens, yes. The
>pool->idle_timer gets queued from an isolated CPU and ends up on a
>housekeeping CPU (cf. get_target_base()).

Yes, you are right.

>With nohz_full on the cmdline, wq_unbound_cpumask already excludes isolated
>CPU, but that doesn't apply to per-CPU kworkers. Or did you mean some other
>mechanism?

Bound kworkers can be destroyed by the idle timer on a housekeeping CPU.

Diff is only for thoughts.

+++ b/kernel/workqueue.c
@@ -1985,6 +1985,7 @@ fail:
static void destroy_worker(struct worker *worker)
{
struct worker_pool *pool = worker->pool;
+ int cpu = smp_processor_id();

lockdep_assert_held(&pool->lock);

@@ -1999,6 +2000,12 @@ static void destroy_worker(struct worker

list_del_init(&worker->entry);
worker->flags |= WORKER_DIE;
+
+ if (!(pool->flags & POOL_DISASSOCIATED) && pool->cpu != cpu) {
+ /* send worker to die on a housekeeping cpu */
+ cpumask_clear(&worker->task->cpus_mask);
+ cpumask_set_cpu(cpu, &worker->task->cpus_mask);
+ }
wake_up_process(worker->task);
}