[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZhAbDkzlKhvHee49@slm.duckdns.org>
Date: Fri, 5 Apr 2024 05:38:54 -1000
From: Tejun Heo <tj@...nel.org>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Leonardo Bras <leobras@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Lai Jiangshan <jiangshanlai@...il.com>,
linux-kernel@...r.kernel.org, Junyao Zhao <junzhao@...hat.com>,
Chris von Recklinghausen <crecklin@...hat.com>,
Frederic Weisbecker <frederic@...nel.org>
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
Powered by blists - more mailing lists