From owner-freebsd-emulation@FreeBSD.ORG Thu Apr 19 09:18:19 2007 Return-Path: X-Original-To: freebsd-emulation@FreeBSD.org Delivered-To: freebsd-emulation@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id CFEC116A40B; Thu, 19 Apr 2007 09:18:19 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailout1.pacific.net.au (mailout1-3.pacific.net.au [61.8.2.210]) by mx1.freebsd.org (Postfix) with ESMTP id 6D42613C484; Thu, 19 Apr 2007 09:18:19 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au [61.8.2.163]) by mailout1.pacific.net.au (Postfix) with ESMTP id F3A4F5BFE2A; Thu, 19 Apr 2007 19:18:17 +1000 (EST) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy2.pacific.net.au (Postfix) with ESMTP id 179EB2740D; Thu, 19 Apr 2007 19:18:15 +1000 (EST) Date: Thu, 19 Apr 2007 19:18:09 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Scot Hetzel In-Reply-To: <790a9fff0704181019m37456143o36c82bd24f7dfe9c@mail.gmail.com> Message-ID: <20070419172414.D4539@delplex.bde.org> References: <790a9fff0612190922t1f4a3fa1m44092944485297f7@mail.gmail.com> <20061219180156.GA87609@stud.fit.vutbr.cz> <790a9fff0704181019m37456143o36c82bd24f7dfe9c@mail.gmail.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Alexander Leidinger , Divacky Roman , freebsd-emulation@FreeBSD.org, jkim@freebsd.org Subject: Re: linuxolator: implement settimeofday call on FreeBSD/amd64 X-BeenThere: freebsd-emulation@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Development of Emulators of other operating systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Apr 2007 09:18:20 -0000 On Wed, 18 Apr 2007, Scot Hetzel wrote: > On 12/19/06, Divacky Roman wrote: >> On Tue, Dec 19, 2006 at 11:22:56AM -0600, Scot Hetzel wrote: >> > I noticed that the settimeofday call in the linuxolator is implemented >> > on FreeBSD/i386, but it is missing from FreeBSD/amd64. The attached >> > patch implements the function on FreeBSD/amd64. >> >> makes me wonder... what is MD on this code? I dont see anything > > I finally figured out what is MD for the settimeofday call. > > On FreeBSD/i386: > > struct l_timeval = struct timeval > > for FreeBSD/amd64 > > struct l_timeval < struct timeval > > The reason for this difference is tv_sec is 32 bits on i386 and 64 > bits on amd64. All handling of this difference (except the definition of l_timeval and the syscall table pointer to a compat function) belongs in compat/linux, where it already is for at least most handling of this difference and the corresponding difference between l_timespec and struct timespec. After committing this, we have the following strange organization and duplication of get/set time handling: - linux_gettimeofday(): implemented in arch/linux (possibly by punning the kern version as on i386's). Buggy on amd64 -- calling the kern version gives 64 bit values and these are blindly truncated to 32-bit values. l_time_t won't overflow until 2038, but when you have a whole function to do the conversion you may as well do the bounds check. - linux_settimeofday(): implemented in arch/linux. Now actually implemented on amd64. Since the conversion expands everything, there is no problem with blind conversion. - linux_clock_gettime(): implemented in compat/linux. The implementation is better than that of gettimeofday() -- the conversions are isolated in functions, and the functions do some range checking. Unfortunately, the range checking is buggy -- it doesn't check that the truncation will work, and it does check that !(tv_sec < 0 || tv_nsec > 999999999), but tv_sec < 0 either can't happen or isn't an error depending on the context. I think tv_nsec < 0 || tv_nsec > 999999 is always an error, but there is no need to check it here provided that the conversion doesn't lose the sign (the range will then be checked by the kern function). Doing the conversion in "MI" code already makes assumptions about the data types so it may as well assume that the sign bit is not lost. For timespecs, the assumptions about the data types are: o l_timespec is a struct with components tv_sec and tv_nsec just like a struct timespec (but perhaps in a different order) (so l_timespec is bogus -- should be struct l_timespec). o there is no padding in l_timespec and/or struct timespec, and/or any padding is harmless. At least the conversion functions are sloppy about doing bzero()s to clear padding. o tv_sec and tv_nsec in l_timespec have type l_time_t and l_suseconds_t, respectively o l_time_t is smaller than time_t and otherwise is the same type (no mixture of signed/unsigned or integer/floating). o l_suseconds_t is smaller than suseconds_t and otherwise is the same type. That's a lot of assumptions, so I think the conversion functions should be MD and l_timeval actually an opaque type. On i386's, clock_gettime(2) is implemented by putting linux_clock_gettime in the syscall table, although putting the kern clock_gettime in the syscall table would work and would be insignificantly more efficient (You could also optimize for space by ifdefing linux_clock_gettime(). Ughly.) - linux_clock_settime(): implemented in compat/linux. As for get/settimeofday(), the conversions are simpler and very unlikely to fail for the "set" variant. Otherwise like clock_gettime(). Strangely, linux_clock_settime() was implemented long before the linux settimeofday(). This seems to be because only the former was obtained from NetBSD (rev.1.1 of linux_time.c) and NetBSD implemented it and linux clock_gettime() more or less correctly. NetBSD also implements *timeofday() in linux_time.c. RELENG_6 doesn't have linux_time.c and seems to be completely missing the linux clock_*time() calls. - freebsd32_{gettimeofday,settimeofday,clock_gettime,clock_settime}(). The amd64 variants could probably use these directly by punning (put them in the syscall table entries). The generic Linux variants are better. freebsd32 seems to be generally sloppy about conversions, and blindly truncates the timevals and timespecs in all of these, using its CP() macro to give smaller source code with no good place to put the range checking. Related style bug: in {amd64,i386}/linux.h, a comment says that l_timespec (alone) is the "stat family of syscalls", but l_timespec isn't even limited to the stat family of syscalls. Bruce