Skip to contentSkip to conversation

kill DECLARE_MUTEX_LOCKED

PATCH In review Christoph Hellwig 3 participants 5 messages Jul 19, 2007 – Jul 20, 2007

Subsystems (2)
USB USB mass storage driver
lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

PATCH 1/1 kill DECLARE_MUTEX_LOCKED

DECLARE_MUTEX_LOCKED was used for semaphores used as completions and we've got rid of them. Well, except for one in libusual that the maintainer explicitly wants to keep as semaphore. So convert that useage to an explicit sema_init and kill of DECLARE_MUTEX_LOCKED so that new code is reminded to use a completion.

Signed-off-by Christoph Hellwig <[email protected]>
include/asm-alpha/semaphore.h +0 -1
Index: linux-2.6/include/asm-alpha/semaphore.h
===================================================================
@@ -30,7 +30,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init(struct semaphore *sem, int val)
 {
include/asm-arm/semaphore.h +0 -1
Index: linux-2.6/include/asm-arm/semaphore.h
===================================================================
@@ -28,7 +28,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INIT(name,count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init(struct semaphore *sem, int val)
 {
include/asm-arm26/semaphore.h +0 -1
Index: linux-2.6/include/asm-arm26/semaphore.h
===================================================================
@@ -29,7 +29,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INIT(name,count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init(struct semaphore *sem, int val)
 {
include/asm-avr32/semaphore.h +0 -1
Index: linux-2.6/include/asm-avr32/semaphore.h
===================================================================
@@ -36,7 +36,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-blackfin/semaphore.h +0 -1
Index: linux-2.6/include/asm-blackfin/semaphore.h
===================================================================
@@ -35,7 +35,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init(struct semaphore *sem, int val)
 {
include/asm-cris/semaphore.h +0 -1
Index: linux-2.6/include/asm-cris/semaphore.h
===================================================================
@@ -35,7 +35,6 @@ struct semaphore {
         struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init(struct semaphore *sem, int val)
 {
include/asm-frv/semaphore.h +0 -1
Index: linux-2.6/include/asm-frv/semaphore.h
===================================================================
@@ -49,7 +49,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-h8300/semaphore.h +0 -1
Index: linux-2.6/include/asm-h8300/semaphore.h
===================================================================
@@ -39,7 +39,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-i386/semaphore.h +0 -1
Index: linux-2.6/include/asm-i386/semaphore.h
===================================================================
@@ -59,7 +59,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-ia64/semaphore.h +0 -1
Index: linux-2.6/include/asm-ia64/semaphore.h
===================================================================
@@ -28,7 +28,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name, count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC(name, 1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC(name, 0)
 
 static inline void
 sema_init (struct semaphore *sem, int val)
include/asm-m32r/semaphore.h +0 -1
Index: linux-2.6/include/asm-m32r/semaphore.h
===================================================================
@@ -35,7 +35,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-m68k/semaphore.h +0 -1
Index: linux-2.6/include/asm-m68k/semaphore.h
===================================================================
@@ -40,7 +40,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init(struct semaphore *sem, int val)
 {
include/asm-m68knommu/semaphore.h +0 -1
Index: linux-2.6/include/asm-m68knommu/semaphore.h
===================================================================
@@ -39,7 +39,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-mips/semaphore.h +0 -1
Index: linux-2.6/include/asm-mips/semaphore.h
===================================================================
@@ -49,7 +49,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC(name, 1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC(name, 0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-parisc/semaphore.h +0 -1
Index: linux-2.6/include/asm-parisc/semaphore.h
===================================================================
@@ -53,7 +53,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 extern inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-powerpc/semaphore.h +0 -1
Index: linux-2.6/include/asm-powerpc/semaphore.h
===================================================================
@@ -35,7 +35,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC(name, 1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC(name, 0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-s390/semaphore.h +0 -1
Index: linux-2.6/include/asm-s390/semaphore.h
===================================================================
@@ -33,7 +33,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-sh/semaphore.h +0 -1
Index: linux-2.6/include/asm-sh/semaphore.h
===================================================================
@@ -37,7 +37,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-sh64/semaphore.h +0 -1
Index: linux-2.6/include/asm-sh64/semaphore.h
===================================================================
@@ -44,7 +44,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-sparc/semaphore.h +0 -1
Index: linux-2.6/include/asm-sparc/semaphore.h
===================================================================
@@ -26,7 +26,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-sparc64/semaphore.h +0 -1
Index: linux-2.6/include/asm-sparc64/semaphore.h
===================================================================
@@ -26,7 +26,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC(name, 1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC(name, 0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-v850/semaphore.h +0 -1
Index: linux-2.6/include/asm-v850/semaphore.h
===================================================================
@@ -22,7 +22,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER (name,count)
 
 #define DECLARE_MUTEX(name)		__DECLARE_SEMAPHORE_GENERIC (name,1)
-#define DECLARE_MUTEX_LOCKED(name)	__DECLARE_SEMAPHORE_GENERIC (name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-x86_64/semaphore.h +0 -1
Index: linux-2.6/include/asm-x86_64/semaphore.h
===================================================================
@@ -60,7 +60,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
include/asm-xtensa/semaphore.h +0 -1
Index: linux-2.6/include/asm-xtensa/semaphore.h
===================================================================
@@ -33,7 +33,6 @@ struct semaphore {
 	struct semaphore name = __SEMAPHORE_INITIALIZER(name,count)
 
 #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
-#define DECLARE_MUTEX_LOCKED(name) __DECLARE_SEMAPHORE_GENERIC(name,0)
 
 static inline void sema_init (struct semaphore *sem, int val)
 {
drivers/usb/storage/libusual.c +3 -1
Index: linux-2.6/drivers/usb/storage/libusual.c
===================================================================
@@ -30,7 +30,7 @@ static atomic_t usu_bias = ATOMIC_INIT(U
 #define BIAS_NAME_SIZE  (sizeof("usb-storage"))
 static const char *bias_names[3] = { "none", "usb-storage", "ub" };
 
-static DECLARE_MUTEX_LOCKED(usu_init_notify);
+static struct semaphore usu_init_notify;
 static DECLARE_COMPLETION(usu_end_notify);
 static atomic_t total_threads = ATOMIC_INIT(0);
 

@@ -204,6 +204,8 @@ static int __init usb_usual_init(void)
 {
 	int rc;
 
+	sema_init(&usu_init_notify, 0);
+
 	rc = usb_register(&usu_driver);
 	up(&usu_init_notify);
 	return rc;
#1 lore raw

Hi Christoph,

Quoted 2 lines · Christoph Hellwig

DECLARE_MUTEX_LOCKED was used for semaphores used as completions and we've got rid of them.

Thanks, I'd been meaning to do this for months :-)
Had even audited the kernel for its usage ... just didn't make or send out a patch, sorry.

Quoted 2 lines

Well, except for one in libusual that the
maintainer explicitly wants to keep as semaphore.

That comment in libusual is quite nonsensical, IMHO. Note that usu_init_notify is declared as DECLARE_MUTEX_LOCKED and yet the author wants us to believe (later, when he's doing that dummy

down();
/* nothing here */
up();

stuff at line no. 181) that the semaphore is somehow "counted" ... Has anybody tried going through all the logic in that file? Looks totally weird to me ... :-) Nothing in there that can't be serialized using proper primitives.

Quoted 3 lines

So convert that
useage to an explicit sema_init and kill of DECLARE_MUTEX_LOCKED so that new code is reminded to use a completion.

and I guess nobody cares a whit for the #if 0'ed stuff in drivers/fc4/fc.c

Quoted 1 line
Signed-off-by Christoph Hellwig <[email protected]>

Well, probably not worth much, but: Ack.

Satyam

author
#2 lore raw
Quoted 3 lines · Satyam Sharma

That comment in libusual is quite nonsensical, IMHO. Note that usu_init_notify is declared as DECLARE_MUTEX_LOCKED and yet the author wants us to believe (later, when he's doing that dummy

Given that no one should use libusual and we should kill it and the abomination called ub I don't really care about fixing this up.

#3 lore raw

Hi,

Quoted 7 lines · Christoph Hellwig
Quoted 3 lines · Satyam Sharma

That comment in libusual is quite nonsensical, IMHO. Note that usu_init_notify is declared as DECLARE_MUTEX_LOCKED and yet the author wants us to believe (later, when he's doing that dummy

Given that no one should use libusual and we should kill it and the abomination called ub I don't really care about fixing this up.

I was actually referring to _this_ "comment":

/* A completion does not work here because it's counted. */

at libusual.c:181, and not yours :-)

It's a bogus comment, because the logic in there (usu_init_notify set to 0 i.e. locked at build time, unlocked to 1 by usb_usual_init() after usb_register() and the the dummy "down(); up();" by the probe threads would mean that the semaphore will *never* ever have a value other than {0, 1}. So contrary to what the author thought, it's not a counted semaphore.

Satyam

#4 lore raw
In reply to Satyam Sharma
Quoted 1 line · On Fri, 20 Jul 2007 10:58:34 +0530 "Satyam Sharma" <[email protected]>…

Well, probably not worth much, but: Ack.

um. The time and effort which you put into reviewing a patch pretty much directly subtracts from the time and effort which I need to put into doing the same.

So: worth lots to me, thanks.