Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 5 Apr 2024 05:38:54 -1000 | From | Tejun Heo <> | Subject | Re: [PATCH v2 1/1] wq: Avoid using isolated cpus' timers on queue_delayed_work |
| |
Hello, Oleg.
On Fri, Apr 05, 2024 at 04:04:49PM +0200, Oleg Nesterov wrote: .. > > > Don't some archs allow the boot CPU to go down too tho? If so, this doesn't > > > really solve the problem, right? > > > > I do not know. But I thought about this too. > > > > In the context of this discussion we do not care if the boot CPU goes down. > > But we need at least one housekeeping CPU after cpu_down(). The comment in > > cpu_down_maps_locked() says > > > > Also keep at least one housekeeping cpu onlined > > > > but it checks HK_TYPE_DOMAIN, and I do not know (and it is too late for me > > to try to read the code ;) if housekeeping.cpumasks[HK_TYPE_TIMER] can get > > empty or not. > > This nearly killed me, but I managed to convince myself we shouldn't worry
Oh no, don't die. :)
> about cpu_down(). > > HK_FLAG_TIMER implies HK_FLAG_TICK. > > HK_FLAG_TICK implies tick_nohz_full_setup() which sets > tick_nohz_full_mask = non_housekeeping_mask. > > When tick_setup_device() is called on a housekeeping CPU it does > > else if (tick_do_timer_boot_cpu != -1 && > !tick_nohz_full_cpu(cpu)) { > tick_take_do_timer_from_boot(); > tick_do_timer_boot_cpu = -1; > > > and this sets tick_do_timer_cpu = first-housekeeping-cpu. > > cpu_down(tick_do_timer_cpu) will fail, tick_nohz_cpu_down() will nack it. > > So cpu_down() can't make housekeeping.cpumasks[HK_FLAG_TIMER] empty and I > still think that the change below is the right approach. > > But probably WARN_ON() in housekeeping_any_cpu() makes sense anyway.
This would be great.
> What do you think? > > Oleg. > > > > > So it seems that we should fix housekeeping_setup() ? see the patch below. > > > > > > > > In any case the usage of cpu_present_mask doesn't look right to me. > > > > > > > > Oleg. > > > > > > > > --- a/kernel/sched/isolation.c > > > > +++ b/kernel/sched/isolation.c > > > > @@ -129,7 +154,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags) > > > > cpumask_andnot(housekeeping_staging, > > > > cpu_possible_mask, non_housekeeping_mask); > > > > > > > > - if (!cpumask_intersects(cpu_present_mask, housekeeping_staging)) { > > > > + if (!cpumask_test_cpu(smp_processor_id(), housekeeping_staging)) { > > > > __cpumask_set_cpu(smp_processor_id(), housekeeping_staging); > > > > __cpumask_clear_cpu(smp_processor_id(), non_housekeeping_mask); > > > > if (!housekeeping.flags) {
Ensuring the boot CPU always be a housekeeping CPU makes sense to me but I'm not very familiar with the housekeeping code. Frederic, what do you think?
Thanks.
-- tejun
| ![\](/images/icornerr.gif) |