linux-kernel - Re: [PATCH 21/23] y2038: itimer: change implementation to timespec64

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Nov 2019 21:06:11 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Arnd Bergmann <arnd@...db.de>, y2038@...ts.linaro.org,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        Anna-Maria Gleixner <anna-maria@...utronix.de>,
        Frederic Weisbecker <frederic@...nel.org>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH 21/23] y2038: itimer: change implementation to
 timespec64

On Wed, 13 Nov 2019 23:28:47 +0100 (CET)
Thomas Gleixner <tglx@...utronix.de> wrote:

> On Fri, 8 Nov 2019, Arnd Bergmann wrote:
> >  TRACE_EVENT(itimer_state,
> >  
> > -	TP_PROTO(int which, const struct itimerval *const value,
> > +	TP_PROTO(int which, const struct itimerspec64 *const value,
> >  		 unsigned long long expires),
> >  
> >  	TP_ARGS(which, value, expires),
> > @@ -321,12 +321,12 @@ TRACE_EVENT(itimer_state,
> >  		__entry->which		= which;
> >  		__entry->expires	= expires;
> >  		__entry->value_sec	= value->it_value.tv_sec;
> > -		__entry->value_usec	= value->it_value.tv_usec;
> > +		__entry->value_usec	= value->it_value.tv_nsec / NSEC_PER_USEC;
> >  		__entry->interval_sec	= value->it_interval.tv_sec;
> > -		__entry->interval_usec	= value->it_interval.tv_usec;
> > +		__entry->interval_usec	= value->it_interval.tv_nsec / NSEC_PER_USEC;  
> 
> Hmm, having a division in a tracepoint is clearly suboptimal.

Right, we should move the division into the TP_printk()

		__entry->interval_nsec = alue->it_interval.tv_nsec;

> 
> >  	),
> >  
> > -	TP_printk("which=%d expires=%llu it_value=%ld.%ld it_interval=%ld.%ld",
> > +	TP_printk("which=%d expires=%llu it_value=%ld.%06ld it_interval=%ld.%06ld",  
> 
> We print only 6 digits after the . so that would be even correct w/o a
> division. But it probably does not matter much.

Well, we still need the division in the printk, otherwise it will print
more than 6. That's just the minimum and it will print the full number.


		__entry->interval_nsec / NSEC_PER_USEC


-- Steve

> 
> > @@ -197,19 +207,13 @@ static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,
> >  #define timeval_valid(t) \
> >  	(((t)->tv_sec >= 0) && (((unsigned long) (t)->tv_usec) < USEC_PER_SEC))  
> 
> Hrm, why do we have yet another incarnation of timeval_valid()? Can we
> please have only one (the inline version)?
> 
> Thanks,
> 
> 	tglx