From owner-svn-src-projects@FreeBSD.ORG Sat Jun 9 15:49:21 2012
Return-Path: <owner-svn-src-projects@FreeBSD.ORG>
Delivered-To: svn-src-projects@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
by hub.freebsd.org (Postfix) with ESMTP id 72C341065672;
Sat, 9 Jun 2012 15:49:21 +0000 (UTC)
(envelope-from mavbsd@gmail.com)
Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com [74.125.82.50])
by mx1.freebsd.org (Postfix) with ESMTP id 717618FC17;
Sat, 9 Jun 2012 15:49:20 +0000 (UTC)
Received: by wgbds11 with SMTP id ds11so1817866wgb.31
for <multiple recipients>; Sat, 09 Jun 2012 08:49:19 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;
h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject
:references:in-reply-to:content-type:content-transfer-encoding;
bh=k5YJaJ3IlPH2pcKgM8pdZpFy7+DbKyUw/JW5MeYC9S4=;
b=QGW70oyi9HlODEbuGBKw4ugVENNmDOFxiS5a26Py0dnxjx6SBsnPZdxrpNjkuurLT4
W33CYYg8ZVnbl5/OByiQ7vIUTn3cvJUcQcXVbqMFsi6bZz8or0yYgWAPQS58bdekSyW7
/n5h/fXh5GX/HWB9qjoGVZB82ve+xsgCeP12PfhGUdmDcaFj1tRwRPtAZQHv016tQeOe
+Vg7ygcLyfZ/OeaWhHQZOxNPrxPTte5OeGNOR3wtbjIRXo+JIK/j5xL5762sE7pvI/4Q
vPrlJ5rRE8TvVYBgPrW25XI3GcAH4Xrt7s33H3wd016SrgZE78Ko6t0Ts8k1TG02zEPc
Ap6g==
Received: by 10.216.214.82 with SMTP id b60mr3372950wep.38.1339256959267;
Sat, 09 Jun 2012 08:49:19 -0700 (PDT)
Received: from mavbook2.mavhome.dp.ua (pc.mavhome.dp.ua. [212.86.226.226])
by mx.google.com with ESMTPS id e20sm10032949wiv.7.2012.06.09.08.49.17
(version=SSLv3 cipher=OTHER); Sat, 09 Jun 2012 08:49:18 -0700 (PDT)
Sender: Alexander Motin <mavbsd@gmail.com>
Message-ID: <4FD3707B.1050407@FreeBSD.org>
Date: Sat, 09 Jun 2012 18:49:15 +0300
From: Alexander Motin <mav@FreeBSD.org>
User-Agent: Mozilla/5.0 (X11; FreeBSD amd64;
rv:12.0) Gecko/20120506 Thunderbird/12.0.1
MIME-Version: 1.0
To: Attilio Rao <attilio@freebsd.org>
References: <201206081153.q58BrqG2056771@svn.freebsd.org>
<CAJ-FndB7QPRnmqrwyR9ixF9Jom8n7P1OMs=rczOKhmRxt08m3g@mail.gmail.com>
In-Reply-To: <CAJ-FndB7QPRnmqrwyR9ixF9Jom8n7P1OMs=rczOKhmRxt08m3g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Davide Italiano <davide@freebsd.org>, svn-src-projects@freebsd.org,
src-committers@freebsd.org
Subject: Re: svn commit: r236744 - in projects/calloutng/sys: kern sys
X-BeenThere: svn-src-projects@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: "SVN commit messages for the src " projects"
tree" <svn-src-projects.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-projects>,
<mailto:svn-src-projects-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-projects>
List-Post: <mailto:svn-src-projects@freebsd.org>
List-Help: <mailto:svn-src-projects-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-projects>,
<mailto:svn-src-projects-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sat, 09 Jun 2012 15:49:21 -0000
On 06/08/12 15:52, Attilio Rao wrote:
> 2012/6/8 Davide Italiano<davide@freebsd.org>:
>> Author: davide
>> Date: Fri Jun 8 11:53:51 2012
>> New Revision: 236744
>> URL: http://svn.freebsd.org/changeset/base/236744
>>
>> Log:
>> Add (experimentally) a function to the sleepqueue(9) KPI
>> sleepq_set_timeout_bt() in which the timeout may be specified in terms
>> of bintime rather than ticks, and which takes advantage of the new
>> precision capabilities of the callout subsystem.
>>
>> Modify the kern_nanosleep() function so that it may rely on
>> sleepq_set_timeout_bt() rather than tsleep().
>>
>> Modified:
>> projects/calloutng/sys/kern/kern_time.c
>> projects/calloutng/sys/kern/subr_sleepqueue.c
>> projects/calloutng/sys/sys/sleepqueue.h
>>
>> Modified: projects/calloutng/sys/kern/kern_time.c
>> ==============================================================================
>> --- projects/calloutng/sys/kern/kern_time.c Fri Jun 8 11:40:30 2012 (r236743)
>> +++ projects/calloutng/sys/kern/kern_time.c Fri Jun 8 11:53:51 2012 (r236744)
>> @@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$");
>> #include<sys/resourcevar.h>
>> #include<sys/signalvar.h>
>> #include<sys/kernel.h>
>> +#include<sys/sleepqueue.h>
>> #include<sys/syscallsubr.h>
>> #include<sys/sysctl.h>
>> #include<sys/sysent.h>
>> @@ -352,37 +353,40 @@ static int nanowait;
>> int
>> kern_nanosleep(struct thread *td, struct timespec *rqt, struct timespec *rmt)
>> {
>> - struct timespec ts, ts2, ts3;
>> - struct timeval tv;
>> - int error;
>> + struct timespec ts;
>> + struct bintime bt, bt2, tmp;
>> + int catch = 0, error;
>>
>> if (rqt->tv_nsec< 0 || rqt->tv_nsec>= 1000000000)
>> return (EINVAL);
>> if (rqt->tv_sec< 0 || (rqt->tv_sec == 0&& rqt->tv_nsec == 0))
>> return (0);
>> - getnanouptime(&ts);
>> - timespecadd(&ts, rqt);
>> - TIMESPEC_TO_TIMEVAL(&tv, rqt);
>> + binuptime(&bt);
>> + timespec2bintime(rqt,&tmp);
>> + bintime_add(&bt,&tmp);
>> for (;;) {
>> - error = tsleep(&nanowait, PWAIT | PCATCH, "nanslp",
>> - tvtohz(&tv));
>> - getnanouptime(&ts2);
>> - if (error != EWOULDBLOCK) {
>> - if (error == ERESTART)
>> - error = EINTR;
>> - if (rmt != NULL) {
>> - timespecsub(&ts,&ts2);
>> - if (ts.tv_sec< 0)
>> - timespecclear(&ts);
>> - *rmt = ts;
>> - }
>> + sleepq_lock(&nanowait);
>> + sleepq_add(&nanowait, NULL, "nanslp", PWAIT | PCATCH, 0);
>> + sleepq_set_timeout_bt(&nanowait,bt);
>> + error = sleepq_timedwait_sig(&nanowait,catch);
>> + binuptime(&bt2);
>> + if (catch) {
>> + if (error != EWOULDBLOCK) {
>> + if (error == ERESTART)
>> + error = EINTR;
>> + if (rmt != NULL) {
>> + tmp = bt;
>> + bintime_sub(&tmp,&bt2);
>> + bintime2timespec(&tmp,&ts);
>> + if (ts.tv_sec< 0)
>> + timespecclear(&ts);
>> + *rmt = ts;
>> + }
>> return (error);
>> + }
>> }
>> - if (timespeccmp(&ts2,&ts,>=))
>> + if (bintime_cmp(&bt2,&bt,>=))
>> return (0);
>> - ts3 = ts;
>> - timespecsub(&ts3,&ts2);
>> - TIMESPEC_TO_TIMEVAL(&tv,&ts3);
>> }
>> }
>>
>>
>> Modified: projects/calloutng/sys/kern/subr_sleepqueue.c
>> ==============================================================================
>> --- projects/calloutng/sys/kern/subr_sleepqueue.c Fri Jun 8 11:40:30 2012 (r236743)
>> +++ projects/calloutng/sys/kern/subr_sleepqueue.c Fri Jun 8 11:53:51 2012 (r236744)
>> @@ -361,6 +361,22 @@ sleepq_add(void *wchan, struct lock_obje
>> * Sets a timeout that will remove the current thread from the specified
>> * sleep queue after timo ticks if the thread has not already been awakened.
>> */
>> +void
>> +sleepq_set_timeout_bt(void *wchan, struct bintime bt)
>> +{
>> +
>> + struct sleepqueue_chain *sc;
>> + struct thread *td;
>> +
>> + td = curthread;
>> + sc = SC_LOOKUP(wchan);
>> + mtx_assert(&sc->sc_lock, MA_OWNED);
>> + MPASS(TD_ON_SLEEPQ(td));
>> + MPASS(td->td_sleepqueue == NULL);
>> + MPASS(wchan != NULL);
>> + callout_reset_bt_on(&td->td_slpcallout, bt, sleepq_timeout, td, PCPU_GET(cpuid));
>> +}
>> +
>
> For this, I'd rather prefer that you patch sleepq_set_timeout() directly to be:
> void sleepq_set_timeout(void *wchan, int timo, struct bintime *bt);
>
> Then, if you pass a NULL ptr to bt the 'timo' is used, otherwise bt is
> given preference in the logic. You will need to patch the current few
> callers of sleepq_set_timo() to get NULL, but it is a small patch.
> I would really like that you do the same also in the callout KPI,
> possibly, because this avoids a lot of code duplication.
As opposite opinion, I don't like an idea of functions with both kinds
of time arguments. It will break existing API/ABI (yes, rarely used, but
already existing and documented) without giving any real benefits.
Duplication of few code lines IMHO is not an argument at all if we are
speaking about choosing API. Speaking about such a popular API as
callout(9), I believe API breakage is just not an option.
--
Alexander Motin