[PATCH.HURD] Limit the minimum timeout delay to 1us for hurdselect.c to avoid problems

[Svante Signell]

Hello,

I'm currently working with an update of hurdselect.c in eglibc, and
found a problem with non-blocking delays of value zero. This problem has
been seen before for vi for select and a workaround hack is currently in
the Debian version of eglibc-2.13. I have also seen this problem with my
new implementation of hurdselect, both for poll() and select(). The
updated code seems to be a little faster for the zero timeout case on my
kvm box, so the problems are revealed with the new version. This problem
is an old one, see Debian bug http://bugs.debian.org/79358.

Attached is test code to check this problem with both poll() and
select() options.
>From the Linux man-page for poll:
"Specifying a timeout of zero causes poll() to return immediately, even
if no file descriptors are ready." '

POSIX.1-2001 specifies ofr poll():
"If the value of timeout is 0, poll() shall return immediately,..."
not saying anything about file descriptors being ready or not.
and for select():
"To effect a poll, the timeout parameter should not be a null pointer,
and should point to a zero-valued timespec structure."

The attached patch is about the same as in Massage 53 of that bug,
limiting the smallest timeout to be 1us. The difference is that it does
not set the timeout to zero if it is given as NULL. This value seems to
work OK both for poll and select cases. Of course this value has been
found by trial and error, but when computers get faster and faster this
value would be an even better bet (unless other problems show up). The
patch is a diff of the patched Debian file.

[Samuel Thibault]

Svante Signell, le Thu 01 Nov 2012 17:32:53 +0100, a écrit :
> --- ../hurdselect_orig.c 2012-10-21 22:55:26.000000000 +0200
> +++ ../hurdselect_orig_timeout.c 2012-11-01 12:58:00.000000000 +0100
> @@ -84,9 +84,13 @@
>
> to = (timeout->tv_sec * 1000 +
> (timeout->tv_nsec + 999999) / 1000000);
> - if (strcmp(program_invocation_short_name, "vi") && strcmp(program_invocation_short_name, "vim") && strcmp(program_invocation_short_name, "vimdiff") && !to)
> - to = 1;
> }
> + /* XXX: A timeout of 0 returns immediately, even if no file
> + descriptors are ready. This is correct according to
> + POSIX.1-2001. As many programs rely on file descriptors being
> + ready for a timeout of zero use 1 msec as the minimum delay */
> + if (to == 0)
> + to = 1;

Nack. We've already tried that in the past, and and using even just 1
makes some applications sluggish like hell. See the archive list for
the current work on actually fixing the issue properly, i.e. adding a
file server interface for probing.

Samuel

[Svante Signell]

Which applications have problems, I would like to verify that. For vim
you have added 1us (or is it 1ms?) as a workaround. The vim the code at
hand seems to be: select(0, NULL, NULL, NULL, &tv) in os_unix.c, i.e. a
pure timeout.

Where in the archives to find the correct implementation ideas, both
bug-hurd and debian-hurd has a long history?

[Samuel Thibault]

Svante Signell, le Thu 01 Nov 2012 18:04:35 +0100, a écrit :
> On Thu, 2012-11-01 at 17:44 +0100, Samuel Thibault wrote:
> > Svante Signell, le Thu 01 Nov 2012 17:32:53 +0100, a écrit :
> > > --- ../hurdselect_orig.c 2012-10-21 22:55:26.000000000 +0200
> > > +++ ../hurdselect_orig_timeout.c 2012-11-01 12:58:00.000000000 +0100
> > > @@ -84,9 +84,13 @@
> > >
> > > to = (timeout->tv_sec * 1000 +
> > > (timeout->tv_nsec + 999999) / 1000000);
> > > - if (strcmp(program_invocation_short_name, "vi") && strcmp(program_invocation_short_name, "vim") && strcmp(program_invocation_short_name, "vimdiff") && !to)
> > > - to = 1;
> > > }
> > > + /* XXX: A timeout of 0 returns immediately, even if no file
> > > + descriptors are ready. This is correct according to
> > > + POSIX.1-2001. As many programs rely on file descriptors being
> > > + ready for a timeout of zero use 1 msec as the minimum delay */
> > > + if (to == 0)
> > > + to = 1;
> >
> > Nack. We've already tried that in the past, and and using even just 1
> > makes some applications sluggish like hell. See the archive list for
> > the current work on actually fixing the issue properly, i.e. adding a
> > file server interface for probing.
>
> Which applications have problems, I would like to verify that.

I don't remember, but it was posing enough problems that we had to use
0.

> For vim you have added 1us (or is it 1ms?) as a workaround.

I have added 1, just like you tried to in your patch.

to = (timeout->tv_sec * 1000

so it's ms.

> Where in the archives to find the correct implementation ideas, both
> bug-hurd and debian-hurd has a long history?

I don't remember. Most probably mails with "select" in the title, of
course.

Samuel

[Svante Signell]

On Thu, 2012-11-01 at 18:08 +0100, Samuel Thibault wrote:
> Svante Signell, le Thu 01 Nov 2012 18:04:35 +0100, a écrit :

> I have added 1, just like you tried to in your patch.
>
> to = (timeout->tv_sec * 1000
>
> so it's ms.

OK, 1ms seems to be a little long. If the time-resolution had been 1us
it could have simplified the problem a lot.

[Richard Braun]

On Thu, Nov 01, 2012 at 06:20:50PM +0100, Svante Signell wrote:
> OK, 1ms seems to be a little long. If the time-resolution had been 1us
> it could have simplified the problem a lot.

This is what my select_timeout branches in some hurd repositories is all
about BTW. But with the recent work on pthreads, which showed other
problems caused by the way select is implemented, I intend to rewrite it
(the big problem is the amount of dead name notifications received by
servers when select is very often used, as is the case in completely
event driven applications).

There is no simple solution to this problem, it requires a lot more
efforts than these ugly hacks.

--
Richard Braun