linux-um archives
 help / color / mirror / Atom feed
From: Jeff Dike <jdike@addtoit.com>
To: Blaisorblade <blaisorblade@yahoo.it>
Cc: user-mode-linux-devel@lists.sourceforge.net,
	Werner Almesberger <wa@almesberger.net>,
	Rob Landley <rob@landley.net>
Subject: Re: [uml-devel] Occasional hang starting up.
Date: Thu, 23 Mar 2006 13:45:51 -0500	[thread overview]
Message-ID: <20060323184551.GA5398@ccure.user-mode-linux.org> (raw)
In-Reply-To: <200603210221.41654.blaisorblade@yahoo.it>

On Tue, Mar 21, 2006 at 02:21:40AM +0100, Blaisorblade wrote:
> However, while I overlooked this, you overlooked what I did:
> *) you must also move the setting of write_sigio_fds and sigio_private above,
> or we'll get the same problem again in different places of 
> write_sigio_thread.

Fixed.

> *) you must update the exit path. I've taken care so that only if everything 
> succeed the result is stored, and to hold the lock for the minimum time 
> needed (particularly not while doing any allocation). I'm not sure whether 
> the first property is needed, but it would be conservative to do so to 
> preserve coherence of data structures.
> 
> In particular: if we store an fd != -1, some function may later close it (say 
> when the console is closed); if we closed it in the exit path, then we're 
> closing an unrelated fd and getting a bug.
> 
> So, if the thread startup fails, you _must_ clear again current_poll, 
> write_sigio_fds and sigio_private.

OK, this is all done, I think.

What do you think about this?

Index: linux-2.6.15/arch/um/os-Linux/sigio.c
===================================================================
--- linux-2.6.15.orig/arch/um/os-Linux/sigio.c	2006-03-23 12:55:27.000000000 -0500
+++ linux-2.6.15/arch/um/os-Linux/sigio.c	2006-03-23 13:41:11.000000000 -0500
@@ -269,42 +269,41 @@ void write_sigio_workaround(void)
 	/* Did we race? Don't try to optimize this, please, it's not so likely
 	 * to happen, and no more than once at the boot. */
 	if(write_sigio_pid != -1)
-		goto out_unlock;
+		goto out_free;
 
-	write_sigio_pid = run_helper_thread(write_sigio_thread, NULL,
-					    CLONE_FILES | CLONE_VM, &stack, 0);
-
-	if (write_sigio_pid < 0)
-		goto out_clear;
+	current_poll = ((struct pollfds) { .poll 	= p,
+					   .used 	= 1,
+					   .size 	= 1 });
 
 	if (write_sigio_irq(l_write_sigio_fds[0]))
-		goto out_kill;
+		goto out_free;
 
-	/* Success, finally. */
 	memcpy(write_sigio_fds, l_write_sigio_fds, sizeof(l_write_sigio_fds));
 	memcpy(sigio_private, l_sigio_private, sizeof(l_sigio_private));
 
-	current_poll = ((struct pollfds) { .poll 	= p,
-					   .used 	= 1,
-					   .size 	= 1 });
+	write_sigio_pid = run_helper_thread(write_sigio_thread, NULL,
+					    CLONE_FILES | CLONE_VM, &stack, 0);
 
-	sigio_unlock();
-	return;
+	if (write_sigio_pid < 0)
+		goto out_clear;
 
- out_kill:
-	l_write_sigio_pid = write_sigio_pid;
-	write_sigio_pid = -1;
 	sigio_unlock();
-	/* Going to call waitpid, avoid holding the lock. */
-	os_kill_process(l_write_sigio_pid, 1);
-	goto out_free;
+	return;
 
  out_clear:
 	write_sigio_pid = -1;
- out_unlock:
-	sigio_unlock();
+	current_poll = ((struct pollfds) { .poll	= NULL,
+					   .size	= 0,
+					   .used	= 0 });
+	write_sigio_fds[0] = -1;
+	write_sigio_fds[1] = -1;
+
+	sigio_private[0] = -1;
+	sigio_private[1] = -1;
+				
  out_free:
 	kfree(p);
+	sigio_unlock();
  out_close2:
 	close(l_sigio_private[0]);
 	close(l_sigio_private[1]);


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

  reply	other threads:[~2006-03-23 18:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-11 18:01 [uml-devel] Occasional hang starting up Rob Landley
2006-03-12 20:33 ` Blaisorblade
2006-03-14 20:22 ` Jeff Dike
2006-03-14 21:31   ` Rob Landley
2006-03-16 14:22   ` Werner Almesberger
2006-03-17 19:36     ` Blaisorblade
2006-03-17 21:27       ` Werner Almesberger
2006-03-17 22:14         ` Blaisorblade
2006-03-17 23:54           ` Werner Almesberger
2006-03-20 10:20       ` Werner Almesberger
2006-03-21  1:00       ` Jeff Dike
2006-03-21  1:21         ` Blaisorblade
2006-03-23 18:45           ` Jeff Dike [this message]
2006-03-23 19:05             ` Werner Almesberger
2006-03-23 20:13               ` Jeff Dike
2006-03-23 21:23                 ` Werner Almesberger
2006-03-23 21:43                   ` Jeff Dike
2006-03-24  0:37             ` Blaisorblade
2006-03-24  2:26               ` Jeff Dike

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060323184551.GA5398@ccure.user-mode-linux.org \
    --to=jdike@addtoit.com \
    --cc=blaisorblade@yahoo.it \
    --cc=rob@landley.net \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    --cc=wa@almesberger.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox