From: Ben Pfaff <blp@cs.stanford.edu>
Date: Sun, 9 Apr 2006 20:19:35 +0000 (+0000)
Subject: The lock functions don't really need to disable interrupts themselves,
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c51576b5aedc01da14c5f29527884f4bea49d544;p=pintos-anon

The lock functions don't really need to disable interrupts themselves,
since they're protected by the semaphore.
Thanks to Godmar Back for reporting this.
---

diff --git a/doc/threads.texi b/doc/threads.texi
index 432cb26..d1ff666 100644
--- a/doc/threads.texi
+++ b/doc/threads.texi
@@ -328,9 +328,10 @@ timer ticks or input events.  Turning off interrupts also increases the
 interrupt handling latency, which can make a machine feel sluggish if
 taken too far.
 
-You may need to add or modify code where interrupts are already
-disabled, such as in @func{sema_up} or @func{sema_down}.  You should
-still try to keep this code as short as you can.
+The synchronization primitives themselves in @file{synch.c} are
+implemented by disabling interrupts.  You may need to increase the
+amount of code that runs with interrupts disabled here, but you should
+still try to keep it to a minimum.
 
 Disabling interrupts can be useful for debugging, if you want to make
 sure that a section of code is not interrupted.  You should remove
diff --git a/src/threads/synch.c b/src/threads/synch.c
index 5ec14ab..c2764bc 100644
--- a/src/threads/synch.c
+++ b/src/threads/synch.c
@@ -192,16 +192,12 @@ lock_init (struct lock *lock)
 void
 lock_acquire (struct lock *lock)
 {
-  enum intr_level old_level;
-
   ASSERT (lock != NULL);
   ASSERT (!intr_context ());
   ASSERT (!lock_held_by_current_thread (lock));
 
-  old_level = intr_disable ();
   sema_down (&lock->semaphore);
   lock->holder = thread_current ();
-  intr_set_level (old_level);
 }
 
 /* Tries to acquires LOCK and returns true if successful or false
@@ -213,18 +209,14 @@ lock_acquire (struct lock *lock)
 bool
 lock_try_acquire (struct lock *lock)
 {
-  enum intr_level old_level;
   bool success;
 
   ASSERT (lock != NULL);
   ASSERT (!lock_held_by_current_thread (lock));
 
-  old_level = intr_disable ();
   success = sema_try_down (&lock->semaphore);
   if (success)
     lock->holder = thread_current ();
-  intr_set_level (old_level);
-
   return success;
 }
 
@@ -236,15 +228,11 @@ lock_try_acquire (struct lock *lock)
 void
 lock_release (struct lock *lock) 
 {
-  enum intr_level old_level;
-
   ASSERT (lock != NULL);
   ASSERT (lock_held_by_current_thread (lock));
 
-  old_level = intr_disable ();
   lock->holder = NULL;
   sema_up (&lock->semaphore);
-  intr_set_level (old_level);
 }
 
 /* Returns true if the current thread holds LOCK, false