Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 02 Sep 2007 23:23:38 -0400
From:      Joe Marcus Clarke <marcus@FreeBSD.org>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        Jilles Tjoelker <jilles@stack.nl>, freebsd-arch@FreeBSD.org
Subject:   Re: Understanding interrupted system calls
Message-ID:  <1188789818.97969.9.camel@shumai.marcuscom.com>
In-Reply-To: <20070902131910.H46281@delplex.bde.org>
References:  <1188600721.1255.11.camel@shumai.marcuscom.com> <20070901112600.GA33832@stack.nl> <1188660782.41727.5.camel@shumai.marcuscom.com> <20070901224025.GA97796@stack.nl> <20070902131910.H46281@delplex.bde.org>

index | next in thread | previous in thread | raw e-mail

[-- Attachment #1 --]
On Sun, 2007-09-02 at 14:17 +1000, Bruce Evans wrote:
> On Sun, 2 Sep 2007, Jilles Tjoelker wrote:
> 
> > On Sat, Sep 01, 2007 at 11:33:02AM -0400, Joe Marcus Clarke wrote:
> >> However, I'm curious as to my other point in this thread.  Why should
> >> one need to re-register the default signal handlers to get a syscall to
> >> return EINTR?  Or should ERESTART be caught and turned into EINTR then
> >> return to the caller (as in kern_connect())?
> >
> > It is intended that most blocking system calls are not interrupted by
> > signals.  This saves the programmer some checks on EINTR.
> 
> Yes, I think this is just the historical default for BSD.  Not
> restarting syscalls is very unusual in BSD, and this was implemented
> by defaulting ps_sigintr to all unset (?).  Before sigaction(2) existed,
> signal(3) was probably signal(2), and most programs used only signal()
> and got the default.  siginterrupt(3) was more probably siginterrupt(2),
> and the few programs that understand this stuff and want to change the
> default had to use it to get EINTR.  Now with sigaction(2), the default
> doesn't apply to userland, since setting up a signal catcher requires
> using sigaction(2) which sets the ps_sigintr bit to the inverse of the
> SA_RESTART bit in the sigaction data.  However, the default might still
> affect operation in the kernel for programs that never call sigaction().
> I think it shouldn't have any effect except to return to near the
> syscall entry point to decide what to do.  Lower levels should see
> ERESTART and return to near the syscall entry point.  Similarly if
> sigaction() is used to change the default.

Thanks, Bruce and Jilles for the explanation.  It is much clearer now.

> 
> > Some system calls, e.g. connect(), read/write/etc that have already
> > committed some data and under BSD also select/poll/kqueue do not restart
> > and always return EINTR or partial success.  In the kernel code, this
> > appears as changing ERESTART to EINTR.
> 
> This translation happens at a high level, but the translation of ps_sigintr
> happens at the [*]sleep() level.  So ps_sigintr has a significant affect
> at a low level, but only (?) to select between ERESTART and EINTR.
> 
> >From Jilles' previous reply:
> 
> >>> The problem seems to be the following code in
> >>> src/sys/dev/syscons/syscons.c, in case VT_WAITACTIVE in scioctl():
> >>> 
> >>> 	while ((error=tsleep(&scp->smode, PZERO|PCATCH,
> >>> 			     "waitvt", 0)) == ERESTART) ;
> >>> 
> >>> If a signal is caught and system call restart is enabled for that
> >>> signal, this makes it spin in a tight loop, waiting in vain for the
> >>> signal to go away.  The idea of ERESTART is that the syscall function
> >>> returns it and then the signal handler is entered.  If and when the
> >>> signal handler returns, it will return to the system call instruction,
> >>> restarting it (perhaps this is optimized to avoid the switch to userland
> >>> and back).  With EINTR, the signal handler would return to directly
> >>> after the system call instruction.
> >>> 
> >>> The fixed version would then be
> >>> 
> >>> 	error = tsleep(&scp->smode, PZERO|PCATCH, "waitvt", 0);
> 
> I think this is right.  The kernel should never loop on ERESTART like this.
> Please fix the remaining style bug in it (missing spaces around binary
> operator).

I think these patches do what you and Jilles suggested.  As Jilles said,
the pcvt patch only applies to -STABLE as this driver was removed in
-CURRENT.  Thanks again, guys, for helping me out with this.

http://www.marcuscom.com/downloads/syscons.c.diff
http://www.marcuscom.com/downloads/pcvt_ext.c.diff

Joe

-- 
Joe Marcus Clarke
FreeBSD GNOME Team      ::      gnome@FreeBSD.org
FreeNode / #freebsd-gnome
http://www.FreeBSD.org/gnome

[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (FreeBSD)

iD8DBQBG2346b2iPiv4Uz4cRAsJwAKCJacuQMP+LUCxYbiWQed5EV6gJAQCfaTFP
cONVTPYttoSv1A3bhmrPGtY=
=d8MY
-----END PGP SIGNATURE-----
home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1188789818.97969.9.camel>