DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
_semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
(which defines real struct mutex). Rename it.
Signed-off-by: Marcin Slusarz <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
---
arch/arm/mach-lh7a40x/clocks.c | 2 +-
drivers/macintosh/adb.c | 2 +-
drivers/staging/go7007/go7007-i2c.c | 4 ++--
include/linux/semaphore.h | 2 +-
kernel/printk.c | 4 ++--
lib/dynamic_printk.c | 2 +-
scripts/checkpatch.pl | 2 +-
sound/soc/s3c24xx/s3c2443-ac97.c | 2 +-
8 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-lh7a40x/clocks.c b/arch/arm/mach-lh7a40x/clocks.c
index 4fb23ac..909ab4e 100644
--- a/arch/arm/mach-lh7a40x/clocks.c
+++ b/arch/arm/mach-lh7a40x/clocks.c
@@ -80,7 +80,7 @@ unsigned int pclkfreq_get (void)
/* ----- */
static LIST_HEAD(clocks);
-static DECLARE_MUTEX(clocks_sem);
+static DEFINE_SEMAPHORE(clocks_sem);
struct clk *clk_get (struct device *dev, const char *id)
{
diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c
index 23741ce..4d9b203 100644
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -83,7 +83,7 @@ static struct adb_driver *adb_controller;
BLOCKING_NOTIFIER_HEAD(adb_client_list);
static int adb_got_sleep;
static int adb_inited;
-static DECLARE_MUTEX(adb_probe_mutex);
+static DEFINE_SEMAPHORE(adb_probe_mutex);
static int sleepy_trackpad;
static int autopoll_devs;
int __adb_probe_sync;
diff --git a/drivers/staging/go7007/go7007-i2c.c b/drivers/staging/go7007/go7007-i2c.c
index cd55b76..0c88e3c 100644
--- a/drivers/staging/go7007/go7007-i2c.c
+++ b/drivers/staging/go7007/go7007-i2c.c
@@ -56,7 +56,7 @@ struct wis_i2c_client_driver {
};
static LIST_HEAD(i2c_client_drivers);
-static DECLARE_MUTEX(i2c_client_driver_list_lock);
+static DEFINE_SEMAPHORE(i2c_client_driver_list_lock);
/* Client drivers register here by their I2C driver ID */
int wis_i2c_add_driver(unsigned int id, found_proc found_proc)
@@ -129,7 +129,7 @@ int wis_i2c_probe_device(struct i2c_adapter *adapter,
/* There is only one I2C port on the TW2804 that feeds all four GO7007 VIPs
* on the Adlink PCI-MPG24, so access is shared between all of them. */
-static DECLARE_MUTEX(adlink_mpg24_i2c_lock);
+static DEFINE_SEMAPHORE(adlink_mpg24_i2c_lock);
static int go7007_i2c_xfer(struct go7007 *go, u16 addr, int read,
u16 command, int flags, u8 *data)
diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
index 7415839..c3657f3 100644
--- a/include/linux/semaphore.h
+++ b/include/linux/semaphore.h
@@ -26,7 +26,7 @@ struct semaphore {
.wait_list = LIST_HEAD_INIT((name).wait_list), \
}
-#define DECLARE_MUTEX(name) \
+#define DEFINE_SEMAPHORE(name) \
struct semaphore name = __SEMAPHORE_INITIALIZER(name, 1)
static inline void sema_init(struct semaphore *sem, int val)
diff --git a/kernel/printk.c b/kernel/printk.c
index 6341af7..132f9b9 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -72,8 +72,8 @@ EXPORT_SYMBOL(oops_in_progress);
* provides serialisation for access to the entire console
* driver system.
*/
-static DECLARE_MUTEX(console_sem);
-static DECLARE_MUTEX(secondary_console_sem);
+static DEFINE_SEMAPHORE(console_sem);
+static DEFINE_SEMAPHORE(secondary_console_sem);
struct console *console_drivers;
EXPORT_SYMBOL_GPL(console_drivers);
diff --git a/lib/dynamic_printk.c b/lib/dynamic_printk.c
index d640f87..a3d154e 100644
--- a/lib/dynamic_printk.c
+++ b/lib/dynamic_printk.c
@@ -34,7 +34,7 @@ static struct hlist_head module_table[DEBUG_HASH_TABLE_SIZE] =
{ [0 ... DEBUG_HASH_TABLE_SIZE-1] = HLIST_HEAD_INIT };
static struct hlist_head module_table2[DEBUG_HASH_TABLE_SIZE] =
{ [0 ... DEBUG_HASH_TABLE_SIZE-1] = HLIST_HEAD_INIT };
-static DECLARE_MUTEX(debug_list_mutex);
+static DEFINE_SEMAPHORE(debug_list_mutex);
/* dynamic_printk_enabled, and dynamic_printk_enabled2 are bitmasks in which
* bit n is set to 1 if any modname hashes into the bucket n, 0 otherwise. They
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f88bb3e..d2e24d6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2428,7 +2428,7 @@ sub process {
}
# check for semaphores used as mutexes
- if ($line =~ /^.\s*(DECLARE_MUTEX|init_MUTEX)\s*\(/) {
+ if ($line =~ /^.\s*(DEFINE_SEMAPHORE|init_MUTEX)\s*\(/) {
WARN("mutexes are preferred for single holder semaphores\n" . $herecurr);
}
# check for semaphores used as mutexes
diff --git a/sound/soc/s3c24xx/s3c2443-ac97.c b/sound/soc/s3c24xx/s3c2443-ac97.c
index 19c5c3c..97dd922 100644
--- a/sound/soc/s3c24xx/s3c2443-ac97.c
+++ b/sound/soc/s3c24xx/s3c2443-ac97.c
@@ -46,7 +46,7 @@ static struct s3c24xx_ac97_info s3c24xx_ac97;
static DECLARE_COMPLETION(ac97_completion);
static u32 codec_ready;
-static DECLARE_MUTEX(ac97_mutex);
+static DEFINE_SEMAPHORE(ac97_mutex);
static unsigned short s3c2443_ac97_read(struct snd_ac97 *ac97,
unsigned short reg)
--
1.5.6.4
On Sun, 2008-10-26 at 13:06 +0100, Marcin Slusarz wrote:
> DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
> _semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
> (which defines real struct mutex). Rename it.
I'd prefer DEFINE_BINARY_SEM or somesuch
Hello,
On Sun, Oct 26, 2008 at 2:39 PM, Peter Zijlstra <[email protected]> wrote:
> On Sun, 2008-10-26 at 13:06 +0100, Marcin Slusarz wrote:
>> DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
>> _semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
>> (which defines real struct mutex). Rename it.
>
> I'd prefer DEFINE_BINARY_SEM or somesuch
>
But it is not a binary semaphore, or is it?
Also, at least under 2.6.24,
#define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
thus the DECLARE is wrong there as well?
Regards,
--
Leon
On Sun, 2008-10-26 at 15:11 +0100, Leon Woestenberg wrote:
> Hello,
>
> On Sun, Oct 26, 2008 at 2:39 PM, Peter Zijlstra <[email protected]> wrote:
> > On Sun, 2008-10-26 at 13:06 +0100, Marcin Slusarz wrote:
> >> DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
> >> _semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
> >> (which defines real struct mutex). Rename it.
> >
> > I'd prefer DEFINE_BINARY_SEM or somesuch
> >
> But it is not a binary semaphore, or is it?
> #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
I thought that 1 made it a binary sem.
Hello,
On Sun, Oct 26, 2008 at 3:41 PM, Peter Zijlstra <[email protected]> wrote:
> On Sun, 2008-10-26 at 15:11 +0100, Leon Woestenberg wrote:
>> On Sun, Oct 26, 2008 at 2:39 PM, Peter Zijlstra <[email protected]> wrote:
>> > On Sun, 2008-10-26 at 13:06 +0100, Marcin Slusarz wrote:
>> >> DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
>> >> _semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
>> >> (which defines real struct mutex). Rename it.
>> >
>> > I'd prefer DEFINE_BINARY_SEM or somesuch
>> >
>> But it is not a binary semaphore, or is it?
>
>> #define DECLARE_MUTEX(name) __DECLARE_SEMAPHORE_GENERIC(name,1)
>
> I thought that 1 made it a binary sem.
>
I thought it only initialized the semaphore count to 1, but not
disallowing it to be upped. Maybe am I just plain wrong, and I should
be zapped to kernelnewbies instead. :-/
Regards,
--
Leon
On Sun, Oct 26, 2008 at 02:39:06PM +0100, Peter Zijlstra wrote:
> On Sun, 2008-10-26 at 13:06 +0100, Marcin Slusarz wrote:
> > DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
> > _semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
> > (which defines real struct mutex). Rename it.
>
> I'd prefer DEFINE_BINARY_SEM or somesuch
Sounds better.
---
Subject: [PATCH] Rename DECLARE_MUTEX to DEFINE_BINARY_SEM
DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
_semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
(which defines real struct mutex). Rename it.
Signed-off-by: Marcin Slusarz <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
---
arch/arm/mach-lh7a40x/clocks.c | 2 +-
drivers/macintosh/adb.c | 2 +-
drivers/staging/go7007/go7007-i2c.c | 4 ++--
include/linux/semaphore.h | 5 +++--
kernel/printk.c | 4 ++--
lib/dynamic_printk.c | 2 +-
scripts/checkpatch.pl | 2 +-
sound/soc/s3c24xx/s3c2443-ac97.c | 2 +-
8 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-lh7a40x/clocks.c b/arch/arm/mach-lh7a40x/clocks.c
index 4fb23ac..de27e81 100644
--- a/arch/arm/mach-lh7a40x/clocks.c
+++ b/arch/arm/mach-lh7a40x/clocks.c
@@ -80,7 +80,7 @@ unsigned int pclkfreq_get (void)
/* ----- */
static LIST_HEAD(clocks);
-static DECLARE_MUTEX(clocks_sem);
+static DEFINE_BINARY_SEM(clocks_sem);
struct clk *clk_get (struct device *dev, const char *id)
{
diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c
index 23741ce..7cf07c2 100644
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -83,7 +83,7 @@ static struct adb_driver *adb_controller;
BLOCKING_NOTIFIER_HEAD(adb_client_list);
static int adb_got_sleep;
static int adb_inited;
-static DECLARE_MUTEX(adb_probe_mutex);
+static DEFINE_BINARY_SEM(adb_probe_mutex);
static int sleepy_trackpad;
static int autopoll_devs;
int __adb_probe_sync;
diff --git a/drivers/staging/go7007/go7007-i2c.c b/drivers/staging/go7007/go7007-i2c.c
index cd55b76..6f698af 100644
--- a/drivers/staging/go7007/go7007-i2c.c
+++ b/drivers/staging/go7007/go7007-i2c.c
@@ -56,7 +56,7 @@ struct wis_i2c_client_driver {
};
static LIST_HEAD(i2c_client_drivers);
-static DECLARE_MUTEX(i2c_client_driver_list_lock);
+static DEFINE_BINARY_SEM(i2c_client_driver_list_lock);
/* Client drivers register here by their I2C driver ID */
int wis_i2c_add_driver(unsigned int id, found_proc found_proc)
@@ -129,7 +129,7 @@ int wis_i2c_probe_device(struct i2c_adapter *adapter,
/* There is only one I2C port on the TW2804 that feeds all four GO7007 VIPs
* on the Adlink PCI-MPG24, so access is shared between all of them. */
-static DECLARE_MUTEX(adlink_mpg24_i2c_lock);
+static DEFINE_BINARY_SEM(adlink_mpg24_i2c_lock);
static int go7007_i2c_xfer(struct go7007 *go, u16 addr, int read,
u16 command, int flags, u8 *data)
diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
index 7415839..824b423 100644
--- a/include/linux/semaphore.h
+++ b/include/linux/semaphore.h
@@ -26,8 +26,9 @@ struct semaphore {
.wait_list = LIST_HEAD_INIT((name).wait_list), \
}
-#define DECLARE_MUTEX(name) \
- struct semaphore name = __SEMAPHORE_INITIALIZER(name, 1)
+#define DEFINE_SEMAPHORE(name, init) \
+ struct semaphore name = __SEMAPHORE_INITIALIZER(name, init)
+#define DEFINE_BINARY_SEM(name) DEFINE_SEMAPHORE(name, 1)
static inline void sema_init(struct semaphore *sem, int val)
{
diff --git a/kernel/printk.c b/kernel/printk.c
index 6341af7..2a2281b 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -72,8 +72,8 @@ EXPORT_SYMBOL(oops_in_progress);
* provides serialisation for access to the entire console
* driver system.
*/
-static DECLARE_MUTEX(console_sem);
-static DECLARE_MUTEX(secondary_console_sem);
+static DEFINE_BINARY_SEM(console_sem);
+static DEFINE_BINARY_SEM(secondary_console_sem);
struct console *console_drivers;
EXPORT_SYMBOL_GPL(console_drivers);
diff --git a/lib/dynamic_printk.c b/lib/dynamic_printk.c
index d640f87..0b3dbdb 100644
--- a/lib/dynamic_printk.c
+++ b/lib/dynamic_printk.c
@@ -34,7 +34,7 @@ static struct hlist_head module_table[DEBUG_HASH_TABLE_SIZE] =
{ [0 ... DEBUG_HASH_TABLE_SIZE-1] = HLIST_HEAD_INIT };
static struct hlist_head module_table2[DEBUG_HASH_TABLE_SIZE] =
{ [0 ... DEBUG_HASH_TABLE_SIZE-1] = HLIST_HEAD_INIT };
-static DECLARE_MUTEX(debug_list_mutex);
+static DEFINE_BINARY_SEM(debug_list_mutex);
/* dynamic_printk_enabled, and dynamic_printk_enabled2 are bitmasks in which
* bit n is set to 1 if any modname hashes into the bucket n, 0 otherwise. They
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f88bb3e..91462b0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2428,7 +2428,7 @@ sub process {
}
# check for semaphores used as mutexes
- if ($line =~ /^.\s*(DECLARE_MUTEX|init_MUTEX)\s*\(/) {
+ if ($line =~ /^.\s*(DEFINE_BINARY_SEM|init_MUTEX)\s*\(/) {
WARN("mutexes are preferred for single holder semaphores\n" . $herecurr);
}
# check for semaphores used as mutexes
diff --git a/sound/soc/s3c24xx/s3c2443-ac97.c b/sound/soc/s3c24xx/s3c2443-ac97.c
index 19c5c3c..8980111 100644
--- a/sound/soc/s3c24xx/s3c2443-ac97.c
+++ b/sound/soc/s3c24xx/s3c2443-ac97.c
@@ -46,7 +46,7 @@ static struct s3c24xx_ac97_info s3c24xx_ac97;
static DECLARE_COMPLETION(ac97_completion);
static u32 codec_ready;
-static DECLARE_MUTEX(ac97_mutex);
+static DEFINE_BINARY_SEM(ac97_mutex);
static unsigned short s3c2443_ac97_read(struct snd_ac97 *ac97,
unsigned short reg)
--
1.5.6.4
On Sun, Oct 26, 2008 at 01:06:14PM +0100, Marcin Slusarz wrote:
> DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
> _semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
> (which defines real struct mutex). Rename it.
NACK. It describes reasonably well what it does. Of course that's a
little confusing as we now have a specialized primitive to just do that.
If the name annoys you enough so that you want to get rid of it help
converting the remaining instances to struct mutex and DEFINE_MUTEX().
On Sun, Oct 26, 2008 at 12:41:18PM -0400, Christoph Hellwig wrote:
> On Sun, Oct 26, 2008 at 01:06:14PM +0100, Marcin Slusarz wrote:
> > DECLARE_MUTEX is doubly misleading name (it actually _defines_ struct
> > _semaphore_ initialized to 1) and it can be confused with DEFINE_MUTEX
> > (which defines real struct mutex). Rename it.
>
> NACK. It describes reasonably well what it does. Of course that's a
> little confusing as we now have a specialized primitive to just do that.
>
> If the name annoys you enough so that you want to get rid of it help
> converting the remaining instances to struct mutex and DEFINE_MUTEX().
Changing the name of DECLARE_MUTEX (to something which tells us what it really
does) will increase visibility of semaphore uses, don't you think?
And more visibility means more patches removing its uses.
Marcin
On Wed, Oct 29, 2008 at 10:16:29PM +0100, Marcin Slusarz wrote:
> Changing the name of DECLARE_MUTEX (to something which tells us what it really
> does) will increase visibility of semaphore uses, don't you think?
> And more visibility means more patches removing its uses.
What it primarily would is cause change churn. Really, if you want to
do something useful try getting rid of it instead.