From owner-svn-src-all@FreeBSD.ORG Mon Jun 6 07:39:59 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4DCFA106564A; Mon, 6 Jun 2011 07:39:59 +0000 (UTC) (envelope-from mm@FreeBSD.org) Received: from mail.vx.sk (mail.vx.sk [IPv6:2a01:4f8:100:1043::3]) by mx1.freebsd.org (Postfix) with ESMTP id C763D8FC13; Mon, 6 Jun 2011 07:39:58 +0000 (UTC) Received: from core.vx.sk (localhost [127.0.0.1]) by mail.vx.sk (Postfix) with ESMTP id 9BA5E170F81; Mon, 6 Jun 2011 09:39:57 +0200 (CEST) X-Virus-Scanned: amavisd-new at mail.vx.sk Received: from mail.vx.sk ([127.0.0.1]) by core.vx.sk (mail.vx.sk [127.0.0.1]) (amavisd-new, port 10024) with LMTP id Npj1Ot1dMmXj; Mon, 6 Jun 2011 09:39:55 +0200 (CEST) Received: from [10.0.3.160] (188-167-50-235.dynamic.chello.sk [188.167.50.235]) by mail.vx.sk (Postfix) with ESMTPSA id 1D5F0170F76; Mon, 6 Jun 2011 09:39:55 +0200 (CEST) Message-ID: <4DEC844A.5090301@FreeBSD.org> Date: Mon, 06 Jun 2011 09:39:54 +0200 From: Martin Matuska User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110424 Thunderbird/3.1.10 MIME-Version: 1.0 To: Andriy Gapon References: <201106040702.p54726O6098336@svn.freebsd.org> <4DEA0785.3040707@FreeBSD.org> <4DEBD5B9.9040908@FreeBSD.org> In-Reply-To: <4DEBD5B9.9040908@FreeBSD.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pawel Jakub Dawidek Subject: Re: svn commit: r222670 - in head/sys/cddl/compat/opensolaris: kern sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jun 2011 07:39:59 -0000 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 >>> 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 >>> >