ChangeSet 1.2255, 2005/03/31 08:35:02-08:00, [EMAIL PROTECTED]

        [PATCH] uml: fix sigio spinlock
        
        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.
        
        Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[EMAIL PROTECTED]>
        Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
        Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]>



 sigio_user.c |   29 ++++++++++++-----------------
 1 files changed, 12 insertions(+), 17 deletions(-)


diff -Nru a/arch/um/kernel/sigio_user.c b/arch/um/kernel/sigio_user.c
--- a/arch/um/kernel/sigio_user.c       2005-03-31 10:16:23 -08:00
+++ b/arch/um/kernel/sigio_user.c       2005-03-31 10:16:23 -08:00
@@ -108,12 +108,14 @@
                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 @@
        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 @@
        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 @@
        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 @@
 
 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:
- */
-
To unsubscribe from this list: send the line "unsubscribe bk-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to