linux-kernel - Re: [PATCH] tick/nohz: Reduce the critical region for jiffies

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: <87o8jxzgj7.fsf@nanos.tec.linutronix.de>
Date:   Mon, 16 Nov 2020 12:29:16 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Yunfeng Ye <yeyunfeng@...wei.com>, fweisbec@...il.com,
        mingo@...nel.org, linux-kernel@...r.kernel.org,
        Shiyuan Hu <hushiyuan@...wei.com>,
        Hewenliang <hewenliang4@...wei.com>
Subject: Re: [PATCH] tick/nohz: Reduce the critical region for jiffies_seq

On Mon, Nov 16 2020 at 14:07, Yunfeng Ye wrote:
> On 2020/11/16 3:43, Thomas Gleixner wrote:
>>> and the conflict between jiffies_lock and jiffies_seq increases,
>>> especially in multi-core scenarios.
>> 
>> This does not make sense. The sequence counter is updated when holding
>> the lock, so there is no conflict between the lock and the sequence
>> count.
>> 
> Yes, there is no conflict between the lock and the sequence count, but
> when tick_do_update_jiffies64() is called one by one, the sequence count
> will be updated, it will affect the latency of tick_nohz_next_event(),
> because the priority of read seqcount is less than writer.

It's clear to me because I know how that code works, but for someone who
is not familiar with it your description of the problem is confusing at
best.

> During a tick period,

'During a tick period' is misleading. The tick period is the reciprocal
value ot the tick frequency.

What you want to explain is that if jiffies require an update, i.e.

     now > last_update + period

then multiple CPUs might contend on it until last_update is forwarded
and the quick check is preventing it again.

> the tick_do_update_jiffies64() is called concurrency, and the
> time is up to 30+us. so the lockless quick check in tick_do_update_jiffies64()
> cannot intercept all concurrency.

It cannot catch all of it, right. 

There are quite some other inefficiencies in that code and the seqcount
held time can be reduced way further. Let me stare at it.

Thanks,

        tglx