Re:[PATCH] Fix poll bug

Re:[PATCH] Fix poll bug

[Guy Bolton King]

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.

> 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. 
> 
> 
> -Andi 
> 
> 
> --- linux/fs/select.c-o Thu Aug 26 02:29:49 1999 
> +++ linux/fs/select.c Sun Feb 13 12:52:00 2000 
> @@ -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) 
> @@ -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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Fix poll bug

[kuznet]

Hello!

> 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.


> 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.


> 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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/