@CWills: yes, this does look like a bug.
Let's look at __lru_add_drain_all():
770 /*
771 * Doesn't need any cpu hotplug locking because we do rely on per-cpu
772 * kworkers being shut down before our page_alloc_cpu_dead callback is
773 * executed on the offlined cpu.
774 * Calling this function with cpu hotplug locks held can actually lead
775 * to obscure indirect dependencies via WQ context.
776 */
777 static inline void __lru_add_drain_all(bool force_all_cpus)
778 {
...
855 cpumask_clear(&has_work);
856 for_each_online_cpu(cpu) {
857 struct work_struct *work = &per_cpu(lru_add_drain_work, cpu);
858
859 if (cpu_needs_drain(cpu)) {
860 INIT_WORK(work, lru_add_drain_per_cpu);
861 queue_work_on(cpu, mm_percpu_wq, work);
862 __cpumask_set_cpu(cpu, &has_work);
863 }
864 }
865
866 for_each_cpu(cpu, &has_work)
867 flush_work(&per_cpu(lru_add_drain_work, cpu));
868
869 done:
870 mutex_unlock(&lock);
871 }
queue_work_on() adds lru_add_drain_per_cpu() to a kworker thread running on each
individual CPU, and as we already know, these kworker threads never get an
opportunity to run until each core has re-entered the kernel / does a syscall /
or sleep.
2491 bool queue_work_on(int cpu, struct workqueue_struct *wq,
2492 struct work_struct *work)
2493 {
2494 bool ret = false;
2495 unsigned long irq_flags;
2496
2497 local_irq_save(irq_flags);
2498
2499 if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work)) &&
2500 !clear_pending_if_disabled(work)) {
2501 __queue_work(cpu, wq, work);
2502 ret = true;
2503 }
2504
2505 local_irq_restore(irq_flags);
2506 return ret;
2507 }
This pretty much just calls __queue_work():
2321 static void __queue_work(int cpu, struct workqueue_struct *wq,
2322 struct work_struct *work)
2323 {
...
2437
2438 /*
2439 * Limit the number of concurrently active work items to max_active.
2440 * @work must also queue behind existing inactive work items to
maintain
2441 * ordering when max_active changes. See wq_adjust_max_active().
2442 */
2443 if (list_empty(&pwq->inactive_works) && pwq_tryinc_nr_active(pwq,
false)) {
2444 if (list_empty(&pool->worklist))
2445 pool->last_progress_ts = jiffies;
2446
2447 trace_workqueue_activate_work(work);
2448 insert_work(pwq, work, &pool->worklist, work_flags);
2449 kick_pool_pick(pool, &wake_task);
2450 } else {
2451 work_flags |= WORK_STRUCT_INACTIVE;
2452 insert_work(pwq, work, &pwq->inactive_works, work_flags);
2453 }
2454
2455 out:
2456 raw_spin_unlock(&pool->lock);
2457 if (wake_task)
2458 wake_up_process(wake_task);
2459 rcu_read_unlock();
2460 }
Okay, there is some interesting things going on in __queue_work(). We either
take the top path, that selects a task, insert_work() adds it to the runqueue,
kick_pool_pick() sets the task to RUNNING and then tries to wake it up.
Which doesn't work, as it won't preempt the SCHED_FIFO userspace task thats
spinning.
The bottom path, just calls insert_work() and places it on the workqeueue for
later processing, a later that never really comes.
I have been trying a couple of ideas.
The first was to see if we are needing to drain the LRU cache on a cpu core
that is isolated, by checking if its not a housekeeping core, and if it isn't
then just send a Inter Processor Interrupt to force the drain to run.
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -33,6 +33,8 @@
#include <linux/page_idle.h>
#include <linux/local_lock.h>
#include <linux/buffer_head.h>
+#include <linux/sched/isolation.h>
+#include <linux/smp.h>
#include "internal.h"
#include "page_alloc.h"
@@ -752,6 +754,11 @@ static void lru_add_drain_per_cpu(struct work_struct
*dummy)
lru_add_and_bh_lrus_drain();
}
+static void lru_add_drain_ipi(void *info)
+{
+ lru_add_and_bh_lrus_drain();
+}
+
static bool cpu_needs_drain(unsigned int cpu)
{
struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu);
@@ -857,9 +864,13 @@ static inline void __lru_add_drain_all(bool force_all_cpus)
struct work_struct *work = &per_cpu(lru_add_drain_work, cpu);
if (cpu_needs_drain(cpu)) {
- INIT_WORK(work, lru_add_drain_per_cpu);
- queue_work_on(cpu, mm_percpu_wq, work);
- __cpumask_set_cpu(cpu, &has_work);
+ if (!housekeeping_cpu(cpu, HK_TYPE_WQ)) {
+ smp_call_function_single(cpu,
lru_add_drain_ipi, NULL, 1);
+ } else {
+ INIT_WORK(work, lru_add_drain_per_cpu);
+ queue_work_on(cpu, mm_percpu_wq, work);
+ __cpumask_set_cpu(cpu, &has_work);
+ }
}
}
I tested this, but it doesn't quite have the performance that I was expecting.
It still gets stuck in D state for too long, and doesn't really improve
anything.
What I am going to try next is to see if I can patch workqueues themselves to
see if I can get it to detect if the work is queued on a CPU that has a higher
priority task, then to send an IPI instead.
I'll let you know how my experiments turn out.
Thanks,
Matthew
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2165410
Title:
Ubuntu LTS 26.04 linux-aws: systemd enters D state and blocks SSH
during cpuset migration on nohz_full CPUs
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux-aws/+bug/2165410/+subscriptions
--
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
