svn commit: r236744 - in projects/calloutng/sys: kern sys
Attilio Rao
attilio at freebsd.org
Fri Jun 8 13:13:24 UTC 2012
2012/6/8 Davide Italiano <davide at freebsd.org>:
> On Fri, Jun 8, 2012 at 2:52 PM, Attilio Rao <attilio at freebsd.org> wrote:
>> 2012/6/8 Davide Italiano <davide at 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);u
>>
>> 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.
>>
>> Attilio
>>
>>
>> --
>> Peace can only be achieved by understanding - A. Einstein
>
> Attilio,
> I agree with you about the few clients of sleepq_set_timeout(), and I
> may change it easily.
> About the callout_* KPI, if you refer to the recently added
> callout_reset_bt_on() function, I have some concerns.
> Right now, the number of consumers of callout_reset() or
> callout_reset_on() is a lot bigger than the one of
> sleepq_set_timeout() to consider a change, at least from my point of
> view.
> Moreover, differently from previous case, callout_reset_bt_on()
> doesn't duplicate code because I've moved the old code there and made
> callout_reset_on() a wrapper in which conversion is made and
> callout_reset_bt_on() is called.
Ok, for the moment just stick with the sleepq change.
About the callout KPI we will see later on once you have a commit candidate.
Attilio
--
Peace can only be achieved by understanding - A. Einstein
More information about the svn-src-projects
mailing list