Changeset View
Standalone View
sys/kern/kern_sig.c
Show First 20 Lines • Show All 1,252 Lines • ▼ Show 20 Lines | |||||
int | int | ||||
kern_sigtimedwait(struct thread *td, sigset_t waitset, ksiginfo_t *ksi, | kern_sigtimedwait(struct thread *td, sigset_t waitset, ksiginfo_t *ksi, | ||||
struct timespec *timeout) | struct timespec *timeout) | ||||
{ | { | ||||
struct sigacts *ps; | struct sigacts *ps; | ||||
sigset_t saved_mask, new_block; | sigset_t saved_mask, new_block; | ||||
struct proc *p; | struct proc *p; | ||||
int error, sig, timo, timevalid = 0; | int error, sig, timevalid = 0; | ||||
struct timespec rts, ets, ts; | sbintime_t sbt, precision, tsbt; | ||||
struct timeval tv; | struct timespec ts; | ||||
bool traced; | bool traced; | ||||
p = td->td_proc; | p = td->td_proc; | ||||
error = 0; | error = 0; | ||||
ets.tv_sec = 0; | |||||
ets.tv_nsec = 0; | |||||
traced = false; | traced = false; | ||||
/* Ensure the sigfastblock value is up to date. */ | /* Ensure the sigfastblock value is up to date. */ | ||||
sigfastblock_fetch(td); | sigfastblock_fetch(td); | ||||
if (timeout != NULL) { | if (timeout != NULL) { | ||||
if (timeout->tv_nsec >= 0 && timeout->tv_nsec < 1000000000) { | if (timeout->tv_nsec >= 0 && timeout->tv_nsec < 1000000000) { | ||||
timevalid = 1; | timevalid = 1; | ||||
getnanouptime(&rts); | ts = *timeout; | ||||
timespecadd(&rts, timeout, &ets); | if (ts.tv_sec < INT32_MAX / 2) { | ||||
tsbt = tstosbt(ts); | |||||
precision = tsbt; | |||||
mav: Why not just: ts.tv_sec = INT32_MAX / 2; ? I don't see over used anywhere else. | |||||
precision >>= tc_precexp; | |||||
if (TIMESEL(&sbt, tsbt)) | |||||
sbt += tc_tick_sbt; | |||||
sbt += tsbt; | |||||
} else | |||||
precision = sbt = 0; | |||||
} | } | ||||
} | } else | ||||
precision = sbt = 0; | |||||
Done Inline Actionsprecision is uninitialized here. Probably not a problem, but not nice. mav: precision is uninitialized here. Probably not a problem, but not nice. | |||||
ksiginfo_init(ksi); | ksiginfo_init(ksi); | ||||
/* Some signals can not be waited for. */ | /* Some signals can not be waited for. */ | ||||
SIG_CANTMASK(waitset); | SIG_CANTMASK(waitset); | ||||
ps = p->p_sigacts; | ps = p->p_sigacts; | ||||
PROC_LOCK(p); | PROC_LOCK(p); | ||||
saved_mask = td->td_sigmask; | saved_mask = td->td_sigmask; | ||||
SIGSETNAND(td->td_sigmask, waitset); | SIGSETNAND(td->td_sigmask, waitset); | ||||
if ((p->p_sysent->sv_flags & SV_SIG_DISCIGN) != 0 || | if ((p->p_sysent->sv_flags & SV_SIG_DISCIGN) != 0 || | ||||
Show All 17 Lines | for (;;) { | ||||
if (error != 0) | if (error != 0) | ||||
break; | break; | ||||
/* | /* | ||||
* POSIX says this must be checked after looking for pending | * POSIX says this must be checked after looking for pending | ||||
* signals. | * signals. | ||||
*/ | */ | ||||
if (timeout != NULL) { | if (timeout != NULL && !timevalid) { | ||||
if (!timevalid) { | |||||
error = EINVAL; | error = EINVAL; | ||||
Done Inline ActionsThis may give false negative due to sbtt += tc_tick_sbt , while the msleep_sbt() below will use precise time. I guess it may end up in a tight loop till next hardclock. mav: This may give false negative due to sbtt += tc_tick_sbt , while the msleep_sbt() below will use… | |||||
break; | break; | ||||
} | } | ||||
getnanouptime(&rts); | |||||
if (timespeccmp(&rts, &ets, >=)) { | |||||
error = EAGAIN; | |||||
break; | |||||
} | |||||
timespecsub(&ets, &rts, &ts); | |||||
TIMESPEC_TO_TIMEVAL(&tv, &ts); | |||||
timo = tvtohz(&tv); | |||||
} else { | |||||
timo = 0; | |||||
} | |||||
if (traced) { | if (traced) { | ||||
error = EINTR; | error = EINTR; | ||||
break; | break; | ||||
} | } | ||||
error = msleep(&p->p_sigacts, &p->p_mtx, PPAUSE | PCATCH, | error = msleep_sbt(&p->p_sigacts, &p->p_mtx, PPAUSE | PCATCH, | ||||
"sigwait", timo); | "sigwait", sbt, precision, C_ABSOLUTE); | ||||
/* The syscalls can not be restarted. */ | /* The syscalls can not be restarted. */ | ||||
if (error == ERESTART) | if (error == ERESTART) | ||||
error = EINTR; | error = EINTR; | ||||
/* We will calculate timeout by ourself. */ | |||||
if (timeout != NULL && error == EAGAIN) | |||||
error = 0; | |||||
/* | /* | ||||
Done Inline ActionsIt would be nice to use this status instead of calling TIMESEL() another time. callouts never return prematurely. mav: It would be nice to use this status instead of calling TIMESEL() another time. callouts never… | |||||
Done Inline Actionsgood catch! btw, as I understand, sbt eq 0 mean that the thread sleep indefinitely, while negative sbt mean thread no sleep. dchagin: good catch!
btw, as I understand, sbt eq 0 mean that the thread sleep indefinitely, while… | |||||
Not Done Inline ActionsMight be callouts indeed do not return prematurely, but note that msleep() uses process-shared wait chain. In other words, if more than one thread called sigtimedwait(2) syscalls, there could be aliased wakeups. Then, if other thread consumed the signal, shouldn't we re-iterate? Or should this case considered an application bug? kib: Might be callouts indeed do not return prematurely, but note that msleep() uses process-shared… | |||||
Not Done Inline ActionsThe removed chunk checked for error == EAGAIN (AKA EWOULDBLOCK) case -- specifically timeout expiration. In case of wakeup() call on the shared wait chain error should be zero and I see the code will indeed reiterate. mav: The removed chunk checked for error == EAGAIN (AKA EWOULDBLOCK) case -- specifically timeout… | |||||
Done Inline ActionsThere will be a storm any way if there are more than one thread, but the threads that do not consume the signal fall to msleep again. dchagin: There will be a storm any way if there are more than one thread, but the threads that do not… | |||||
* If PTRACE_SCE or PTRACE_SCX were set after | * If PTRACE_SCE or PTRACE_SCX were set after | ||||
* userspace entered the syscall, return spurious | * userspace entered the syscall, return spurious | ||||
* EINTR after wait was done. Only do this as last | * EINTR after wait was done. Only do this as last | ||||
* resort after rechecking for possible queued signals | * resort after rechecking for possible queued signals | ||||
* and expired timeouts. | * and expired timeouts. | ||||
*/ | */ | ||||
if (error == 0 && (p->p_ptevents & PTRACE_SYSCALL) != 0) | if (error == 0 && (p->p_ptevents & PTRACE_SYSCALL) != 0) | ||||
traced = true; | traced = true; | ||||
▲ Show 20 Lines • Show All 3,035 Lines • Show Last 20 Lines |
Why not just: ts.tv_sec = INT32_MAX / 2; ? I don't see over used anywhere else.