linux-kernel - Re: [RFC PATCH] hrtimer: interleave timers for improved single thread performance at low utilization

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: <7116a13a-6566-3cff-50ef-137d8ed0c7e4@linux.vnet.ibm.com>
Date:   Tue, 31 Jan 2023 17:57:32 +0530
From:   shrikanth hegde <sshegde@...ux.vnet.ibm.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     peterz@...radead.org, arjan@...ux.intel.com, mingo@...nel.org,
        Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
        svaidy@...ux.ibm.com, linux-kernel@...r.kernel.org,
        bigeasy@...utronix.de
Subject: Re: [RFC PATCH] hrtimer: interleave timers for improved single thread
 performance at low utilization



On 1/31/23 4:38 PM, Thomas Gleixner wrote:
> On Tue, Jan 31 2023 at 11:18, shrikanth hegde wrote:
>> As per current design of hrtimer, it uses the _softexpires to trigger the
>> timer function.  _softexpires is set as multiple of the period/interval value.
> 
> Wrong. _softexpires is _hardexpires + slack. The slack allows for
> batching which:
> 
Ok. understood.
>> This will benefit the power saving by less wakeups.
> 
This is what i thought was the rationale of _softexpires
> But that has absolutely nothing to do with your problem:
> 
Yes. 
>> Due to this, different timers of the same period/interval values align
>> and the callbacks functions will be called at the same time.
> 
> The whole point of hrtimer_forward_now() is to forward the expiry time
> of a timer with the given period so that it expires after 'now'.
> 
> That's functionality which is used by a lot of callers to implement
> proper periodic timers.
> 
>> Came up with a naive patch, more of hack.
> 
> A broken hack to be precise because any existing user of
> hrtimer_forward() will be broken by this hack.
> 
Agree. Aim was to describe the problem. 
>> Other alternative is to use a slightly modified API for cgroups, so
>> that all other timers align and wakeups remain reduced.
> 
> I'm not seeing why you need a new API for that. The problem is _NOT_ in
> the hrtimer code at all.
> 
> Lets look at the math:
> 
>     expiry = $INITIAL_EXPIRYVALUE + $N * $PERIOD
> 
> If $INITIAL_EXPIRYVALUE is the same then for all instances then
> obviously the expiry values of all instances will be all aligned on
> multiples of $PERIOD, right?
> 
> So why the heck do you need a new hrtimer API? There is an obvious
> solution, right?
> 
> Thanks,
> 
>         tglx

Thanks Thomas for taking a look and correcting wrong bits here.  The problem    
here was that $INITIAL_EXPIRYVALUE was never set. Hence the _softexpires was    
always set as the Multiple of $PERIOD.                                          
                                                                                
As Ingo Suggested this, this can be done in sched code. would need to set the    
$INITIAL_EXPIRYVALUE in start_cfs_bandwidth and Timers would interleave on the  
same logic which was described in the patch.                                    
                                                                                
No change in hrtimer code  would be needed. will send out the patch after some  
more testing.