From owner-freebsd-arch@FreeBSD.ORG Sun Dec 2 13:19:41 2007 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B022916A418 for ; Sun, 2 Dec 2007 13:19:41 +0000 (UTC) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (xorpc.icir.org [192.150.187.68]) by mx1.freebsd.org (Postfix) with ESMTP id 85FE413C447 for ; Sun, 2 Dec 2007 13:19:41 +0000 (UTC) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (localhost [127.0.0.1]) by xorpc.icir.org (8.12.11/8.13.6) with ESMTP id lB2CpZPs007701; Sun, 2 Dec 2007 04:51:35 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.11/8.12.3/Submit) id lB2CpZKY007700; Sun, 2 Dec 2007 04:51:35 -0800 (PST) (envelope-from rizzo) Date: Sun, 2 Dec 2007 04:51:35 -0800 From: Luigi Rizzo To: Poul-Henning Kamp Message-ID: <20071202045134.A7421@xorpc.icir.org> References: <15391.1196547545@critter.freebsd.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <15391.1196547545@critter.freebsd.dk>; from phk@phk.freebsd.dk on Sat, Dec 01, 2007 at 10:19:05PM +0000 Cc: arch@freebsd.org Subject: Re: New "timeout" api, to replace callout X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Dec 2007 13:19:41 -0000 On Sat, Dec 01, 2007 at 10:19:05PM +0000, Poul-Henning Kamp wrote: > > Here is my proposed new timeout API for 8.x. ... remaining limited to the client API, I have the following questions: > /* > * A duration of time, represented in the optimal way for a given provider > * or family of providers (ie: per cpu). > */ > typedef int timeout_time; is this meant to be parsable by client code, or should it be opaque (even though of known size) ? > /* > * Flag values, > * can be used as return from the function or as argument to timeout_arm() > */ > #define TIMEOUT_REARM (1<<0) ... any reason not to use an enum for these ? > /* > * Convert various human compatible time-units to internal units > * Since these are potentially expensive (as in multiple integer divisions) > * caching the return value is adviced for heavy use. > * Choice of function indicates level of precision requested, so > * timeout_ms_time(1000) may be different from timeout_s_time(1), depending > * on the implementation. > */ > timeout_time timeout_ns_time(struct timeout_p *, unsigned nsec); > timeout_time timeout_us_time(struct timeout_p *, unsigned usec); > timeout_time timeout_ms_time(struct timeout_p *, unsigned msec); > timeout_time timeout_s_time(struct timeout_p *, unsigned sec); Two questions here: 1. is there any need for the first argument not to be const ? If all the function do is a conversion, there shouldn't be any need to modify the timeout_p 2. I fully agree that the precision level should be a user-supplied parameter, but wonder whether ns/us/ms/s is really the set of precisions one might want. I could see a need for at least the following requests: "as accurate as possible" "one timecounter tick accuracy, whatever the tick is" "one timer tick (1/HZ) accuracy, whatever the tick is" " So how about moving to a slightly different API to convert timeouts back and forth between Human and Internal representation e.g timeout_time timeout_UtoI(const struct timeout_p *, int units, enum timeout_precision); with enum timeout_precision { TP_NANOSECOND, TP_MICROSECOND, TP_MILLISECOND, TP_SECOND, TP_DEFAULT, /* default: units is us */ TP_HIGHEST, /* highest possible; units is in ns */ TP_TIMECOUNTER, /* one timecounter tick */ TP_TIMECOUNTER, /* one timecounter tick */ ... } The 'unit' argument is in whatever unit applies to the precision, i.e. ns, us, ms, ticks, ... when obvious, or we need to specify a suitable value for TP_DEFAULT and TP_HIGHEST We might also make use of the inverse conversion, something like int timeout_ItoU(const struct timeout_p *, timeout_time t, enum timeout_precision); cheers luigi