The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: blaisorblade@yahoo.it
To: torvalds@osdl.org
Cc: akpm@osdl.org, jdike@addtoit.com, linux-kernel@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net,
	blaisorblade@yahoo.it
Subject: [patch 1/8] uml: fix sigio spinlock [for 2.6.12]
Date: Wed, 30 Mar 2005 19:33:41 +0200	[thread overview]
Message-ID: <20050330173341.41F25EFED3@zion> (raw)


I just saw a "take twice spinlock" deadlock with the Spinlock debugging
enabled on this lock, and static code analysis revealed this is the culprit:
update_thread can take (in an error path) the sigio_lock, which is already
held by all its callers (it's a static function, so it's easy to verify).

Added some comments to mark where this function needs the lock, in case
someone wants to reduce the locking here.

Also clean an exitcall to mark the thread as killed (won't hurt, and could
be useful if things go wrong).

As a bonus, some CodingStyle cleanups.

This should go in 2.6.12 for its simplicity and usefulness.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---

 linux-2.6.11-paolo/arch/um/kernel/sigio_user.c |   29 ++++++++++---------------
 1 files changed, 12 insertions(+), 17 deletions(-)

diff -puN arch/um/kernel/sigio_user.c~uml-fix-sigio-spinlock arch/um/kernel/sigio_user.c
--- linux-2.6.11/arch/um/kernel/sigio_user.c~uml-fix-sigio-spinlock	2005-03-24 11:37:00.000000000 +0100
+++ linux-2.6.11-paolo/arch/um/kernel/sigio_user.c	2005-03-24 11:37:00.000000000 +0100
@@ -108,12 +108,14 @@ static void tty_output(int master, int s
 		panic("check_sigio : write failed, errno = %d\n", errno);
 	while(((n = os_read_file(slave, buf, sizeof(buf))) > 0) && !got_sigio) ;
 
-	if(got_sigio){
+	if (got_sigio) {
 		printk("Yes\n");
 		pty_output_sigio = 1;
+	} else if (n == -EAGAIN) {
+		printk("No, enabling workaround\n");
+	} else {
+		panic("check_sigio : read failed, err = %d\n", n);
 	}
-	else if(n == -EAGAIN) printk("No, enabling workaround\n");
-	else panic("check_sigio : read failed, err = %d\n", n);
 }
 
 static void tty_close(int master, int slave)
@@ -235,6 +237,8 @@ static int need_poll(int n)
 	return(0);
 }
 
+/* Must be called with sigio_lock held, because it's needed by the marked
+ * critical section. */
 static void update_thread(void)
 {
 	unsigned long flags;
@@ -257,7 +261,7 @@ static void update_thread(void)
 	set_signals(flags);
 	return;
  fail:
-	sigio_lock();
+	/* Critical section start */
 	if(write_sigio_pid != -1) 
 		os_kill_process(write_sigio_pid, 1);
 	write_sigio_pid = -1;
@@ -265,7 +269,7 @@ static void update_thread(void)
 	os_close_file(sigio_private[1]);
 	os_close_file(write_sigio_fds[0]);
 	os_close_file(write_sigio_fds[1]);
-	sigio_unlock();
+	/* Critical section end */
 	set_signals(flags);
 }
 
@@ -418,19 +422,10 @@ int read_sigio_fd(int fd)
 
 static void sigio_cleanup(void)
 {
-	if(write_sigio_pid != -1)
+	if (write_sigio_pid != -1) {
 		os_kill_process(write_sigio_pid, 1);
+		write_sigio_pid = -1;
+	}
 }
 
 __uml_exitcall(sigio_cleanup);
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only.  This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
_

                 reply	other threads:[~2005-03-30 18:52 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20050330173341.41F25EFED3@zion \
    --to=blaisorblade@yahoo.it \
    --cc=akpm@osdl.org \
    --cc=jdike@addtoit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    --cc=user-mode-linux-devel@lists.sourceforge.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