linux-kernel - Re: [PATCH 1/5] sched,time: count actually elapsed irq & softirq time

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]

Message-ID: <20160628205641.GA24297@lerouge>
Date:	Tue, 28 Jun 2016 22:56:43 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Rik van Riel <riel@...hat.com>
Cc:	linux-kernel@...r.kernel.org, peterz@...radead.org,
	mingo@...nel.org, pbonzini@...hat.com, fweisbec@...hat.com,
	wanpeng.li@...mail.com, efault@....de, tglx@...utronix.de,
	rkrcmar@...hat.com
Subject: Re: [PATCH 1/5] sched,time: count actually elapsed irq & softirq time

On Mon, Jun 27, 2016 at 08:50:06AM -0400, Rik van Riel wrote:
> On Mon, 2016-06-27 at 14:25 +0200, Frederic Weisbecker wrote:
> > > 
> > >   * Accumulate raw cputime values of dead tasks (sig->[us]time) and
> > > live
> > >   * tasks (sum on group iteration) belonging to @tsk's group.
> > >   */
> > > @@ -344,19 +378,24 @@ static void
> > > irqtime_account_process_tick(struct task_struct *p, int user_tick,
> > >  {
> > >  	cputime_t scaled = cputime_to_scaled(cputime_one_jiffy);
> > >  	u64 cputime = (__force u64) cputime_one_jiffy;
> > > -	u64 *cpustat = kcpustat_this_cpu->cpustat;
> > > +	unsigned long other;
> > >  
> > > -	if (steal_account_process_tick(ULONG_MAX))
> > > +	/*
> > > +	 * When returning from idle, many ticks can get accounted
> > > at
> > > +	 * once, including some ticks of steal, irq, and softirq
> > > time.
> > > +	 * Subtract those ticks from the amount of time accounted
> > > to
> > > +	 * idle, or potentially user or system time. Due to
> > > rounding,
> > > +	 * other time can exceed ticks occasionally.
> > > +	 */
> > > +	other = account_other_ticks(ticks);
> > > +	if (other >= ticks)
> > >  		return;
> > > +	ticks -= other;
> > >  
> > >  	cputime *= ticks;
> > >  	scaled *= ticks;
> > So instead of dealing with ticks here, I think you should rather use
> > the above
> > cputime as both the limit and the remaining time to account after
> > steal/irqs.
> > 
> > This should avoid some middle conversions and improve precision when
> > cputime_t == nsecs granularity.
> > 
> > If we account 2 ticks to idle (lets say HZ=100) and irq time to
> > account is 15 ms. 2 ticks = 20 ms
> > so we have 5 ms left to account to idle. With the jiffies granularity
> > in this patch, we would account
> > one tick to irqtime (1 tick = 10 ms, there will be 5 ms to account
> > back later) and one tick to idle
> > time whereas if you deal with cputime_t, you are going to account the
> > correct amount of idle time.
> > 
> 
> Ahhh, so you want irqtime_account_process_tick to work with
> and account fractional ticks when calling account_system_time,
> account_user_time, account_idle_time, etc?

Not exactly. This function is passed ticks but works with the cputime
converted value of those ticks. And it would be more precise to work on
top of cputime values without converting things to jiffies in the middle
as we may lose precision on the way.

> 
> I guess that should work fine since we already pass cputime
> values in, anyway.
> 
> I suppose we can do the same for get_vtime_delta, too.

But get_vtime_delta() already returns cputime, right?

Thanks.