Re: [PATCH resent] Documentation: rw_lock lessons learned

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Stephen Hemminger
Date: Tuesday, November 10, 2009 - 7:06 pm

On Tue, 10 Nov 2009 14:55:44 -0500
William Allen Simpson <william.allen.simpson@gmail.com> wrote:


I would rather see the text in Documentation/spinlocks give an explaination
as to why reader/writer locks are normally not desirable. 

The whole document needs work to make it a developer document, rather than
a historical mail thread..  A good document says what should be done today,
and does not have old junk or ask the reader to overly new context
on old information.



--- a/Documentation/spinlocks.txt	2009-11-10 17:47:03.801984416 -0800
+++ b/Documentation/spinlocks.txt	2009-11-10 18:01:00.779749888 -0800
@@ -1,73 +1,8 @@
-SPIN_LOCK_UNLOCKED and RW_LOCK_UNLOCKED defeat lockdep state tracking and
-are hence deprecated.
+Lesson 1: Spin locks
 
-Please use DEFINE_SPINLOCK()/DEFINE_RWLOCK() or
-__SPIN_LOCK_UNLOCKED()/__RW_LOCK_UNLOCKED() as appropriate for static
-initialization.
-
-Most of the time, you can simply turn:
-
-	static spinlock_t xxx_lock = SPIN_LOCK_UNLOCKED;
-
-into:
-
-	static DEFINE_SPINLOCK(xxx_lock);
-
-Static structure member variables go from:
-
-	struct foo bar {
-		.lock	=	SPIN_LOCK_UNLOCKED;
-	};
-
-to:
-
-	struct foo bar {
-		.lock	=	__SPIN_LOCK_UNLOCKED(bar.lock);
-	};
-
-Declaration of static rw_locks undergo a similar transformation.
-
-Dynamic initialization, when necessary, may be performed as
-demonstrated below.
-
-   spinlock_t xxx_lock;
-   rwlock_t xxx_rw_lock;
-
-   static int __init xxx_init(void)
-   {
-   	spin_lock_init(&xxx_lock);
-	rwlock_init(&xxx_rw_lock);
-	...
-   }
-
-   module_init(xxx_init);
-
-The following discussion is still valid, however, with the dynamic
-initialization of spinlocks or with DEFINE_SPINLOCK, etc., used
-instead of SPIN_LOCK_UNLOCKED.
-
------------------------
-
-On Fri, 2 Jan 1998, Doug Ledford wrote:
-> 
-> I'm working on making the aic7xxx driver more SMP friendly (as well as
-> importing the latest FreeBSD sequencer code to have 7895 support) and wanted
-> to get some info from you.  The goal here is to make the various routines
-> SMP safe as well as UP safe during interrupts and other manipulating
-> routines.  So far, I've added a spin_lock variable to things like my queue
-> structs.  Now, from what I recall, there are some spin lock functions I can
-> use to lock these spin locks from other use as opposed to a (nasty)
-> save_flags(); cli(); stuff; restore_flags(); construct.  Where do I find
-> these routines and go about making use of them?  Do they only lock on a
-> per-processor basis or can they also lock say an interrupt routine from
-> mucking with a queue if the queue routine was manipulating it when the
-> interrupt occurred, or should I still use a cli(); based construct on that
-> one?
-
-See <asm/spinlock.h>. The basic version is:
-
-   spinlock_t xxx_lock = SPIN_LOCK_UNLOCKED;
+The most basic primitive for locking is spinlock.
 
+static DEFINE_SPINLOCK(xxx_lock);
 
 	unsigned long flags;
 
@@ -141,13 +76,17 @@ Lesson 2: reader-writer spinlocks.
 
 If your data accesses have a very natural pattern where you usually tend
 to mostly read from the shared variables, the reader-writer locks
-(rw_lock) versions of the spinlocks are often nicer. They allow multiple
+(rw_lock) versions of the spinlocks are sometimes useful. They allow multiple
 readers to be in the same critical region at once, but if somebody wants
-to change the variables it has to get an exclusive write lock. The
-routines look the same as above:
+to change the variables it has to get an exclusive write lock.
+
+NOTE! reader-writer locks require more atomic memory operations than
+simple spinlocks, so unless the reader critical secition is long you
+are better off just using spinlocks.
 
-   rwlock_t xxx_lock = RW_LOCK_UNLOCKED;
+The routines look the same as above:
 
+static DEFINE_RWLOCK(xxx_lock);
 
 	unsigned long flags;
 
@@ -159,12 +98,15 @@ routines look the same as above:
 	.. read and write exclusive access to the info ...
 	write_unlock_irqrestore(&xxx_lock, flags);
 
-The above kind of lock is useful for complex data structures like linked
+The above kind of lock might useful for complex data structures like linked
 lists etc, especially when you know that most of the work is to just
 traverse the list searching for entries without changing the list itself,
 for example. Then you can use the read lock for that kind of list
 traversal, which allows many concurrent readers. Anything that _changes_
-the list will have to get the write lock. 
+the list will have to get the write lock.
+
+NOTE! RCU is better for that most read only access, but requires
+correct operations (see Documentation/RCU/listRCU.txt)
 
 Note: you cannot "upgrade" a read-lock to a write-lock, so if you at _any_
 time need to do any changes (even if you don't do it every time), you have
@@ -233,4 +175,45 @@ indeed), while write-locks need to prote
 
 		Linus
 
+Reference information:
+
+* Older code used SPIN_LOCK_UNLOCKED and RW_LOCK_UNLOCKED to initialize
+  locks, but this is now deprecated because it interferes with the
+  lockdep state tracking. Please use DEFINE_SPINLOCK()/DEFINE_RWLOCK() or
+  __SPIN_LOCK_UNLOCKED()/__RW_LOCK_UNLOCKED() as appropriate for static
+  initialization.
+
+  Most of the time, you can simply turn:
+	static spinlock_t xxx_lock = SPIN_LOCK_UNLOCKED;
+  into:
+	static DEFINE_SPINLOCK(xxx_lock);
+
+  Static structure member variables go from:
+
+	struct foo bar {
+		.lock	=	SPIN_LOCK_UNLOCKED;
+	};
+
+  to:
+
+	struct foo bar {
+		.lock	=	__SPIN_LOCK_UNLOCKED(bar.lock);
+	};
+
+  Declaration of static rw_locks undergo a similar transformation.
+
+  Dynamic initialization, when necessary, may be performed as
+  demonstrated below.
+
+   spinlock_t xxx_lock;
+   rwlock_t xxx_rw_lock;
+
+   static int __init xxx_init(void)
+   {
+   	spin_lock_init(&xxx_lock);
+	rwlock_init(&xxx_rw_lock);
+	...
+   }
+
+   module_init(xxx_init);
 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH resent] Documentation: rw_lock lessons learned, William Allen Simpson, (Tue Nov 10, 12:55 pm)
Re: [PATCH resent] Documentation: rw_lock lessons learned, Paul E. McKenney, (Tue Nov 10, 2:22 pm)
Re: [PATCH resent] Documentation: rw_lock lessons learned, Stephen Hemminger, (Tue Nov 10, 7:06 pm)
Re: [PATCH resent] Documentation: rw_lock lessons learned, William Allen Simpson, (Wed Nov 11, 10:08 am)
Re: [PATCH resent] Documentation: rw_lock lessons learned, Stephen Hemminger, (Wed Nov 11, 10:37 am)
[PATCH v2] Documentation: rw_lock lessons learned, William Allen Simpson, (Thu Nov 12, 4:06 am)
Re: [PATCH v2] Documentation: rw_lock lessons learned, Linus Torvalds, (Thu Nov 12, 8:40 am)
Re: [PATCH v2] Documentation: rw_lock lessons learned, Stephen Hemminger, (Thu Nov 12, 10:04 am)
Re: [PATCH v2] Documentation: rw_lock lessons learned, Stephen Clark, (Thu Nov 12, 12:13 pm)
Re: [PATCH v2] Documentation: rw_lock lessons learned, Stephen Hemminger, (Thu Nov 12, 4:00 pm)
Documentation: rw_lock lessons learned, Stefan Richter, (Fri Nov 13, 1:59 am)
Re: Documentation: rw_lock lessons learned, William Allen Simpson, (Fri Nov 13, 9:15 am)
Re: [PATCH v2] Documentation: rw_lock lessons learned, William Allen Simpson, (Fri Dec 11, 10:01 am)
Re: [PATCH v2] Documentation: rw_lock lessons learned, Jarek Poplawski, (Fri Dec 11, 2:07 pm)
Re: [PATCH v2] Documentation: rw_lock lessons learned, William Allen Simpson, (Sat Dec 12, 3:36 am)