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 <d...@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
>>>  
>
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to