svn commit: r222670 - in head/sys/cddl/compat/opensolaris: kern sys

[Andriy Gapon]

on 04/06/2011 13:23 Martin Matuska said the following:
> If we are "fixing" this,

I am not sure why you used the quotes here.  I think that it's a real issue and
I believe that this commit is a reasonable fix.

> what about following pjd's rules and moving in
> direction vendor code (reducing diff)? Or is it too expensive?

Well, it seems that the code before the change was not compliant with the rules.
I certainly like those rules, but maybe in this case they would be an overkill.

> If that way:
> 
> a) the nsec_per_tick is defined in vendor code
> "usr/src/uts/common/conf/param.c".
> 
> Maybe we should define it at least in
> sys/cddl/compat/opensolaris/sys/param.h and not in time.h
> 
> b) sys/cddl/compat/opensolaris/kern/opensolaris_sunddi.c
> might contain the functions as in vendor code (uts/common/os/sunddi.c):
> 
> clock_t
> ddi_get_lbolt(void)
> {
> 	return ((clock_t)lbolt_hybrid());
> }
> 
> int64_t
> ddi_get_lbolt64(void)
> {
> 	return (lbolt_hybrid());
> }
> 
> c) sys/cddl/compat/opensolaris/sys/time.h:
> 
> extern clock_t	ddi_get_lbolt(void);
> extern int64_t	ddi_get_lbolt64(void);
> 
> d) we might want a new file called
> sys/cddl/compat/opensolaris/kern/opensolaris_clock.c
> (uts/common/os/clock.c):
> 
> int64_t
> lbolt_hybrid(void)
> {
> 	return (gethrtime() / nsec_per_tick);
> }
> 
> The d) step with lbolt_hybrid might be omitted and the function result
> integrated directly into opensolaris_sunddi.c. Bud regardless of that,
> notice the clock_t cast in ddi_get_lbolt().
> 
> mm
> 
> Dňa 04.06.2011 09:02, Andriy Gapon  wrote / napísal(a):
>> Author: avg
>> Date: Sat Jun  4 07:02:06 2011
>> New Revision: 222670
>> URL: http://svn.freebsd.org/changeset/base/222670
>>
>> Log:
>>   opensolaris compat / zfs: avoid early overflow in ddi_get_lbolt*
>>   
>>   Reported by:	David P. Discher <dpd at bitgravity.com>
>>   Tested by:	will
>>   Reviewed by:	art
>>   Discussed with:	dwhite
>>   MFC after:	2 weeks
>>
>> Modified:
>>   head/sys/cddl/compat/opensolaris/kern/opensolaris.c
>>   head/sys/cddl/compat/opensolaris/sys/time.h
>>
>> Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris.c
>> ==============================================================================
>> --- head/sys/cddl/compat/opensolaris/kern/opensolaris.c	Sat Jun  4 04:35:12 2011	(r222669)
>> +++ head/sys/cddl/compat/opensolaris/kern/opensolaris.c	Sat Jun  4 07:02:06 2011	(r222670)
>> @@ -40,6 +40,7 @@
>>  cpu_core_t	cpu_core[MAXCPU];
>>  kmutex_t	cpu_lock;
>>  solaris_cpu_t	solaris_cpu[MAXCPU];
>> +int		nsec_per_tick;
>>  
>>  /*
>>   *  OpenSolaris subsystem initialisation.
>> @@ -60,6 +61,8 @@ opensolaris_load(void *dummy)
>>  	}
>>  
>>  	mutex_init(&cpu_lock, "OpenSolaris CPU lock", MUTEX_DEFAULT, NULL);
>> +
>> +	nsec_per_tick = NANOSEC / hz;
>>  }
>>  
>>  SYSINIT(opensolaris_register, SI_SUB_OPENSOLARIS, SI_ORDER_FIRST, opensolaris_load, NULL);
>>
>> Modified: head/sys/cddl/compat/opensolaris/sys/time.h
>> ==============================================================================
>> --- head/sys/cddl/compat/opensolaris/sys/time.h	Sat Jun  4 04:35:12 2011	(r222669)
>> +++ head/sys/cddl/compat/opensolaris/sys/time.h	Sat Jun  4 07:02:06 2011	(r222670)
>> @@ -62,8 +62,21 @@ gethrtime(void) {
>>  #define	gethrestime(ts)		getnanotime(ts)
>>  #define	gethrtime_waitfree()	gethrtime()
>>  
>> -#define	ddi_get_lbolt()		((gethrtime() * hz) / NANOSEC)
>> -#define	ddi_get_lbolt64()	(int64_t)((gethrtime() * hz) / NANOSEC)
>> +extern int nsec_per_tick;	/* nanoseconds per clock tick */
>> +
>> +static __inline int64_t
>> +ddi_get_lbolt64(void)
>> +{
>> +
>> +	return (gethrtime() / nsec_per_tick);
>> +}
>> +
>> +static __inline clock_t
>> +ddi_get_lbolt(void)
>> +{
>> +
>> +	return (ddi_get_lbolt64());
>> +}
>>  
>>  #else
>>  


-- 
Andriy Gapon