All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.