Skip to contentSkip to conversation

Fix poll bug

PATCH Guy Bolton King 2 participants 2 messages Dec 14, 2000

lore
Share & tools
lore

Guy Bolton King

author
#0 lore raw

Hi,

I believe I've found an inconsistency between the behaviour of poll(2) and select(2); select() is restartable in the face of signals (sys_select() returns ERESTARTNOHAND if a signal is pending), whilst poll() is not (sys_poll() returns EINTR). On Feb 13 2000, Andi Kleen posted a patch for 2.2.14--2.2.15 that fixes this (reproduced below; the explanation is somewhat confusing and doesn't match my problem, but the patch does The Right Thing), but it hasn't gone into any of the subsequent 2.2 series.

Is this deliberate? And if so, why (because it's just bitten me quite badly)? If not, I assume this has been fixed for 2.4? I note that this problem (i.e. restartable system calls) has cropped up a few times on the list, always with poll(2), usually in combination with job control or gdb, so it _does_ appear to be confusing users and breaking code (or at least breaking code that expects BSD signal behaviour as the default, which I believe is also the POSIX way---please correct me if I'm wrong about this last one).

Guy.

Quoted 45 lines

Hi Alan,

I just found a poll() bug. poll currently returns -EINTR when
a signal occurs, which causes the system call to be restarted. Unfortunately the timeout is not decreased, so when you have a periodic SA_RESTART timer with just the right period and no other event occurs poll hangs forever.

select does it correctly, it always returns EINTR and is never restarted.

I found it because syslogd's 30s "mark" timer can trigger it in the dns resolver when syslogd does forwarding.

This patch fixes the problem. I also fixed an outdated comment.

Please add for 2.2.15. Thanks.

(quoted diff) +0 -1
-Andi
fs/select.c Sun Feb 13 12:52:00 2000 renamed from fs/select.c-o Thu Aug 26 02:29:49 1999 +2 -6
@@ -230,12 +230,8 @@ 
 } 
 
 /* 
- * We can actually return ERESTARTSYS instead of EINTR, but I'd 
- * like to be certain this leads to no problems. So I return 
- * EINTR just for safety. 
- * 
- * Update: ERESTARTSYS breaks at least the xview clock binary, so 
- * I'm trying ERESTARTNOHAND which restart only when you want to. 
+ * On interrupts return ERESTARTNOHAND, because automatic restart 
+ * is not handled properly (the timeout is not decreased). 
  */ 
 #define MAX_SELECT_SECONDS \ 
         ((unsigned long) (MAX_SCHEDULE_TIMEOUT / HZ)-1)

fs/select.c Sun Feb 13 12:52:00 2000 line 436

Quoted 8 lines
fs/select.c Sun Feb 13 12:52:00 2000 renamed from fs/select.c-o Thu Aug 26 02:29:49 1999 +1 -1
@@ -434,7 +430,7 @@ 
 
         err = fdcount; 
         if (!fdcount && signal_pending(current)) 
- err = -EINTR; 
+ err = -ERESTARTNOHAND; 
 
 out_fds: 
         kfree(fds);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#1 lore raw

Hello!

Quoted 4 lines

I believe I've found an inconsistency between the behaviour of poll(2) and select(2); select() is restartable in the face of signals (sys_select() returns ERESTARTNOHAND if a signal is pending), whilst poll() is not (sys_poll() returns EINTR).

poll() cannot be restarted.

select() can, because state of timer is remembered.

Quoted 2 lines

breaking code (or at least breaking code that expects BSD signal behaviour as the default,

BSD behaviour is never to restart select() (because BSD does not update timer, it is the only possible behaviour there, otherwise select can never terminate). Its restart in some rare cases (when signal happens to be ignored) is small optimization, specific only to Linux.

Quoted 1 line

which I believe is also the POSIX

Seems, POSIX behaviour is not to restart any syscall.

Alexey
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/