lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 15 Dec 2020 18:52:49 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Peter Zijlstra <peterz@...radead.org>,
        Steven Rostedt <rostedt@...dmis.org>
Cc:     Anna-Maria Behnsen <anna-maria@...utronix.de>,
        linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        Daniel Bristot de Oliveira <bristot@...hat.com>
Subject: Re: [PATCH] sched: Prevent raising SCHED_SOFTIRQ when CPU is !active

On Tue, Dec 15 2020 at 16:05, Peter Zijlstra wrote:
> On Tue, Dec 15, 2020 at 09:34:15AM -0500, Steven Rostedt wrote:
>> On Tue, 15 Dec 2020 15:23:39 +0100 (CET)
>> Anna-Maria Behnsen <anna-maria@...utronix.de> wrote:
>> 
>> > > > +	/*
>> > > > +	 * Remove CPU from nohz.idle_cpus_mask to prevent participating in
>> > > > +	 * load balancing when not active
>> > > > +	 */
>> > > > +	nohz_balance_exit_idle(rq);
>> > > > +
>> > > >  	set_cpu_active(cpu, false);
>> > > >  	/*
>> > > >  	 * We've cleared cpu_active_mask, wait for all preempt-disabled and RCU  
>> > > 
>> > > OK, so we must clear the state before !active, because getting an
>> > > interrupt/softirq after would trigger the badness. And we're guaranteed
>> > > nothing blocks between them to re-set it.  
>> > 
>> > As far as I understood, it is not a problem whether the delete is before or
>> > after !active. When it is deleted after, the remote CPU will return in
>> > kick_ilb() because cpu is not idle, because it is running the hotplug
>> > thread.
>> 
>> I was thinking that disabling it after may also cause some badness. Even if
>> it does not, I think there's no harm in clearing it just before setting cpu
>> active to false. And I find that the safer option.
>
> The paranoid in me wanted to write it like:
>
> 	preempt_disable();
> 	nohz_balance_exit_idle(rq);
> 	set_cpu_active(cpu, false);
> 	preempt_enable();
>
> (or possibly even local_irq_disable), to guarantee we don't hit idle
> between them (which could re-set the nohz idle state we just cleared).
>
> But then I gave up :-)

I might be missing something, but how is the CPU which runs the pinned
kernel thread, i.e. the hotplug thread, supposed to go idle between the
two calls?

Really the order is completely irrelevant.

  Remote kick_ilb() checks nohz_mask _AND_ idle_cpu()

  Local nohz_enter() checks cpu_active()

I still might be missing something magic though, mushrooms perhaps. :)

Thanks,

        tglx

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ