Date: Thu, 19 Apr 2007 19:18:09 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Scot Hetzel <swhetzel@gmail.com> Cc: Alexander Leidinger <Alexander@leidinger.net>, Divacky Roman <rdivacky@FreeBSD.org>, freebsd-emulation@FreeBSD.org, jkim@freebsd.org Subject: Re: linuxolator: implement settimeofday call on FreeBSD/amd64 Message-ID: <20070419172414.D4539@delplex.bde.org> In-Reply-To: <790a9fff0704181019m37456143o36c82bd24f7dfe9c@mail.gmail.com> References: <790a9fff0612190922t1f4a3fa1m44092944485297f7@mail.gmail.com> <20061219180156.GA87609@stud.fit.vutbr.cz> <790a9fff0704181019m37456143o36c82bd24f7dfe9c@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 18 Apr 2007, Scot Hetzel wrote:
> On 12/19/06, Divacky Roman <xdivac02@stud.fit.vutbr.cz> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070419172414.D4539>