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

[Martin Matuska]

The quotes don't mean the fix isn't complete - it is complete.
I was just discussing the way we put it in the code and comparing how it
looks like upstream (so if there are any code updates in the future it
should be easier to integrate them).
Btw. pjd is ok with this commit as the vendor code did not get altered
so you may ignore my thoughts.

Cheers,
mm

Dňa 05.06.2011 21:15, Andriy Gapon  wrote / napísal(a):
> 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
>>>  
>