linux-kernel - Re: [tip:sched/core] sched: Fix granularity of task

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]

Date:	Fri, 13 Nov 2009 10:40:29 +0100
From:	Stanislaw Gruszka <sgruszka@...hat.com>
To:	mingo@...hat.com, hpa@...or.com, spencer@...ehost.com,
	linux-kernel@...r.kernel.org, seto.hidetoshi@...fujitsu.com,
	peterz@...radead.org, tglx@...utronix.de, oleg@...hat.com,
	mingo@...e.hu
Cc:	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:sched/core] sched: Fix granularity of task_u/stime()

On Thu, Nov 12, 2009 at 06:12:41PM +0000, tip-bot for Hidetoshi Seto wrote:
> sched: Fix granularity of task_u/stime()
> 
> Originally task_s/utime() were designed to return clock_t but
> later changed to return cputime_t by following commit:
> 
>   commit efe567fc8281661524ffa75477a7c4ca9b466c63
>   Author: Christian Borntraeger <borntraeger@...ibm.com>
>   Date:   Thu Aug 23 15:18:02 2007 +0200
> 
> It only changed the type of return value, but not the
> implementation. As the result the granularity of task_s/utime()
> is still that of clock_t, not that of cputime_t.
>
> So using task_s/utime() in __exit_signal() makes values
> accumulated to the signal struct to be rounded and coarse
> grained.
>
> This patch removes casts to clock_t in task_u/stime(), to keep
> granularity of cputime_t over the calculation.

Patch make only difference on 64 arch and do not fix the ordinal utime
decreasing problem on 32 bits. Anyway except below nit looks good
for me (but I to be honest I don't understand this {u/s}time adjusting
functions at all).

> -	p->prev_utime = max(p->prev_utime, clock_t_to_cputime(utime));
> +	p->prev_utime = max(p->prev_utime, utime);
>  	return p->prev_utime;
>  }
>  
>  cputime_t task_stime(struct task_struct *p)
>  {
> -	clock_t stime;
> +	cputime_t stime;
>  
>  	/*
>  	 * Use CFS's precise accounting. (we subtract utime from
>  	 * the total, to make sure the total observed by userspace
>  	 * grows monotonically - apps rely on that):
>  	 */
> -	stime = nsec_to_clock_t(p->se.sum_exec_runtime) -
> -			cputime_to_clock_t(task_utime(p));
> +	stime = nsecs_to_cputime(p->se.sum_exec_runtime) - task_utime(p);
>  
>  	if (stime >= 0)

Since cputime_t is not signed type, we should just remove this or
write something like that:

	runtime = nsecs_to_cputime(p->se.sum_exec_runtime);
	utime = task_utime(p);
	if (cputime_ge(runtime, utime))
		p->prev_stime = max(p->prev_stime, runtime - utime);

Stanislaw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/