On Fri, Jul 18, 2008 at 11:58 AM, Jilles Tjoelker <[EMAIL PROTECTED]> wrote: > On Sat, Jul 12, 2008 at 07:11:26PM -0400, Michael B Allen wrote: >> Below is a semtimedop(2) implementation that I'm using for FreeBSD. I >> was hoping someone could look it over and tell me if they think the >> implementation is sound. > >> [snip semtimedop implementation that uses SIGALRM and relies on EINTR] > >> The code seems to work ok but when stressing the FreeBSD build of my app >> I have managed to provoke errors related to concurrency (usually when a >> SIGALRM goes off). The Linux build works flawlessesly so I'm wondering >> about this one critical function that is different. > > In your implementation, the SIGALRM signal may happen before you even > call semop(2). If so, most likely the semop(2) will hang arbitrarily > long.
Indeed. And I reconnoiter that condition is likely to happen if called with a sufficiently small timeout value. > Another dirty fix is to try non-blocking semop(2) several times with > sleeps in between. Actually that seems to work pretty well. If I force the nanosleep codepath to be used always, the application actually works pretty well under load. I wonder if the overhead of managing the signal and timer is worth the trouble. For posterity, below is the current implementation of semtimedop(2) for FreeBSD. Further ideas are welcome. void _timeval_diff(struct timeval *tv1, struct timeval *tv2, struct timeval *tvd) { tvd->tv_sec = tv1->tv_sec - tv2->tv_sec; tvd->tv_usec = tv1->tv_usec - tv2->tv_usec; if (tvd->tv_usec < 0) { tvd->tv_usec = 1000000 - tvd->tv_usec; tvd->tv_sec--; } } void signal_ignore(int s, siginfo_t *si, void *ctx) { } int _semtimedop(int semid, struct sembuf *array, size_t nops, struct timespec *_timeout) { struct timeval timeout, before, after; struct itimerval value, ovalue; struct sigaction sa, osa; int ret; if (_timeout) { timeout.tv_sec = _timeout->tv_sec; timeout.tv_usec = _timeout->tv_nsec / 1000; if (gettimeofday(&before, NULL) == -1) { errno = EFAULT; return -1; } if (timeout.tv_sec == 0 && timeout.tv_usec < 5000) { struct timeval tsleep, tend; struct sembuf wait; wait = *array; wait.sem_flg |= IPC_NOWAIT; tsleep.tv_sec = 0; tsleep.tv_usec = 1; timeradd(&before, &timeout, &tend); for ( ;; ) { struct timeval tnow, tleft; struct timespec ts; ret = semop(semid, &wait, nops); if (ret == 0) { return 0; } else if (errno != EAGAIN) { break; } if (gettimeofday(&tnow, NULL) == -1) { errno = EFAULT; break; } if (timercmp(&tnow, &tend, >=)) { errno = EAGAIN; break; } timersub(&tend, &tnow, &tleft); if (tsleep.tv_usec > tleft.tv_usec) tsleep.tv_usec = tleft.tv_usec; ts.tv_sec = 0; ts.tv_nsec = tsleep.tv_usec * 1000; nanosleep(&ts, NULL); tsleep.tv_usec *= 10; } return -1; } memset(&value, 0, sizeof value); value.it_value = timeout; memset(&sa, 0, sizeof sa); sa.sa_sigaction = signal_ignore; sa.sa_flags = SA_SIGINFO; sigemptyset(&sa.sa_mask); sigaction(SIGALRM, &sa, &osa); if (setitimer(ITIMER_REAL, &value, &ovalue) == -1) { sigaction(SIGALRM, &osa, NULL); return -1; } } ret = semop(semid, array, nops); if (_timeout) { ret = setitimer(ITIMER_REAL, &ovalue, NULL); if (ret < 0) errno = EFAULT; sigaction(SIGALRM, &osa, NULL); } if (ret == -1) { if (_timeout) { struct timeval elapsed; if (gettimeofday(&after, NULL) == -1) { } else { _timeval_diff(&after, &before, &elapsed); if (timercmp(&elapsed, &timeout, >=)) errno = EAGAIN; } } return -1; } return 0; } _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"