linux-kernel - Re: [patch 7/8] hrtimer: Avoid unnecessary SMP function calls in clock_was

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] [day] [month] [year] [list]

Date:   Sat, 15 May 2021 02:24:55 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Anna-Maria Behnsen <anna-maria@...utronix.de>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Peter Xu <peterx@...hat.com>,
        Nitesh Narayan Lal <nitesh@...hat.com>,
        Alex Belits <abelits@...vell.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        John Stultz <john.stultz@...aro.org>
Subject: Re: [patch 7/8] hrtimer: Avoid unnecessary SMP function calls in clock_was_set()

On Sat, May 15 2021 at 01:28, Peter Zijlstra wrote:

> On Fri, May 14, 2021 at 08:52:33PM +0200, Thomas Gleixner wrote:
>> On Thu, May 13 2021 at 16:59, Peter Zijlstra wrote:
>> > On Tue, Apr 27, 2021 at 10:25:44AM +0200, Thomas Gleixner wrote:
>> >> -	/* Retrigger the CPU local events everywhere */
>> >> -	on_each_cpu(retrigger_next_event, NULL, 1);
>> >> +	if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) {
>> >> +		on_each_cpu(retrigger_next_event, NULL, 1);
>> >
>> > This will violate NOHZ_FULL;
>> 
>> Only if that allocation fails.
>
> Right, which should be near to never I suppose.
>
>> Aside of that any CPU which has an affected timer will get notified even
>> on NOHZ_FULL.
>
> Right; but if it's properly NOHZ_FULL -- the kind that wanted a signal
> on any entry into the kernel -- when it won't have timers and this IPI
> will trigger the signal and kill the program.

That's true today. clock_was_set() IPI's unconditionally. The whole
point of this exercise is to avoid that if there are no armed timers on
the affected clocks.

>> >> +	preempt_disable();
>> >> +	smp_call_function_many(mask, retrigger_next_event, NULL, 1);
>> >
>> > The sane option is:
>> >
>> > 	smp_call_function_many_cond(cpu_online_mask, retrigger_next_event,
>> > 				    NULL, SCF_WAIT, update_needs_ipi);
>> >
>> > Which does all of the above, but better.
>> 
>> With the difference that the for_each_cpu() loop runs with preemption
>> disabled, while with this approach preemption is only disabled accross
>> the function call.
>
> Yeah, I'd forgotten that... I might put looking at that on the todo list
> somewhere :/

That's this append only thingy, right?

Thanks,

        tglx