Re: [uml-devel] Occasional hang starting up.

[Blaisorblade <blaisorblade@yahoo.it>]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On Thursday 16 March 2006 15:22, Werner Almesberger wrote:
> Jeff Dike wrote:
> > Having said that,
> > if you can get a stack from the hang, it might turn out to be an easily
> > diagnosed and fixed bug.
>
> Seems that arch/um/kernel/sigio_user.c:write_sigio_thread begins
> its poll before there are any fds in current_poll

Ah, and you mean it hangs...

> , so update_thread 
> caused by a add_sigio_fd shortly thereafter hangs while waiting for
> a response.

Clearly

> A sleep(1) at the beginning of write_sigio_thread "fixes" this :-)

Er... there's only one thing which seems unclear - poll(NULL, 0, -1) shouldn't 
fail with -EFAULT instead of hanging? Yep, it should. I didn't believe what 
you said, so. But then I went to check the implementation. And, indeed, it 
seems that Linux isn't up to this :-).

So:

1) I'll try to fix poll(2) to return -EINVAL. Dunno whether anyone will say 
"no, the app is stupid, it deserves no error", but hope not (with "try" I 
refer to this). Attached patch should do this.

2) write_sigio_thread should do a "down" on a semaphore/mutex and the first 
update_thread should "up" it. As usually, this semaphore would be indeed 
implemented as a pipe.
-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

[-- Attachment #2: poll-mustn_t-hang-0-fds.patch --]
[-- Type: text/x-diff, Size: 1062 bytes --]

poll(2): don't hang caller if nfds == 0

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>, Werner Almesberger <wa@almesberger.net>

As Werner Almesberger observed in a real program, poll(2) will never return if
called with nfds == 0; which is stupid but not so badly deserving.

I looked at the code and verified that indeed such a case is never detected and
do_poll keeps looping, checking any fd passed, overlooking it wasn't passed any
and calling schedule_timeout() endlessly. At least it's a TASK_INTERRUPTIBLE
sleep...

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
Index: linux-2.6.git/fs/select.c
===================================================================
--- linux-2.6.git.orig/fs/select.c
+++ linux-2.6.git/fs/select.c
@@ -629,6 +629,10 @@ int do_sys_poll(struct pollfd __user *uf
 	struct fdtable *fdt;
 	int max_fdset;
 
+	/* Otherwise below code would hang! */
+	if (unlikely(nfds < 1))
+		return -EINVAL;
+
 	/* Do a sanity check on nfds ... */
 	rcu_read_lock();
 	fdt = files_fdtable(current->files);