linux-kernel - Re: [PATCH] hrtimer: avoid retrigger_next

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]

Message-ID: <87lf9q4lue.ffs@nanos.tec.linutronix.de>
Date:   Sat, 10 Apr 2021 09:53:13 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Marcelo Tosatti <mtosatti@...hat.com>,
        Anna-Maria Behnsen <anna-maria@...utronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Frederic Weisbecker <frederic@...nel.org>,
        Peter Xu <peterx@...hat.com>,
        Nitesh Narayan Lal <nitesh@...hat.com>,
        Alex Belits <abelits@...vell.com>
Subject: Re: [PATCH] hrtimer: avoid retrigger_next_event IPI

On Fri, Apr 09 2021 at 13:51, Marcelo Tosatti wrote:
> On Fri, Apr 09, 2021 at 04:15:13PM +0200, Thomas Gleixner wrote:
>> On Wed, Apr 07 2021 at 10:53, Marcelo Tosatti wrote:
>> ---> fail because that newly started timer is on the old offset.
>
> CPU0								CPU1
>
>
> clock_was_set()
> 							Case-1: CPU-1 grabs base->lock before CPU-0:
> 							CPU-0 sees active_mask[CPU1] and IPIs.
>
> 							base = lock_hrtimer_base(timer, &flags);
>         						if (__hrtimer_start_range_ns(timer, tim, ...
>                 						hrtimer_reprogram(timer, true);
>
>         						unlock_hrtimer_base(timer, &flags);
>
>
> raw_spin_lock_irqsave(&cpu_base->lock, flags);
> if (need_reprogram_timer(cpu_base))
>         cpumask_set_cpu(cpu, mask);
> else
>         hrtimer_update_base(cpu_base);
> raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
>
> 							Case-2: CPU-1 grabs base->lock after CPU-0:
> 							CPU-0 will have updated the offsets remotely.
>
> 							base = lock_hrtimer_base(timer, &flags);
>         						if (__hrtimer_start_range_ns(timer, tim, ...
>                 						hrtimer_reprogram(timer, true);
>
>         						unlock_hrtimer_base(timer, &flags);
>
>
> No?

Yeah, you're right. I misread the loop logic.

Can we please make that unconditional independent of nohz full. There is
no reason to special case it.

Thanks,

        tglx