Date: Sun, 05 Jun 2011 22:15:05 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Martin Matuska <mm@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pawel Jakub Dawidek <pjd@FreeBSD.org> Subject: Re: svn commit: r222670 - in head/sys/cddl/compat/opensolaris: kern sys Message-ID: <4DEBD5B9.9040908@FreeBSD.org> In-Reply-To: <4DEA0785.3040707@FreeBSD.org> References: <201106040702.p54726O6098336@svn.freebsd.org> <4DEA0785.3040707@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DEBD5B9.9040908>