* [patch 1/2] echoaudio semaphore to mutex
[not found] <20071209171509.601451827@gmail.com>
@ 2007-12-09 17:15 ` Kevin Winchester
2007-12-10 9:06 ` Giuliano Pochini
2007-12-17 10:15 ` Takashi Iwai
2007-12-09 17:15 ` [patch 2/2] 9p util " Kevin Winchester
1 sibling, 2 replies; 5+ messages in thread
From: Kevin Winchester @ 2007-12-09 17:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Ingo Molnar, Giuliano Pochini, Jaroslav Kysela, linux-kernel,
Kevin Winchester
[-- Attachment #1: echoaudio-sem-to-mutex.patch --]
[-- Type: text/plain, Size: 3533 bytes --]
Convert the semaphore to a mutex in echoaudio.c
Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
---
sound/pci/echoaudio/echoaudio.c | 18 +++++++++---------
sound/pci/echoaudio/echoaudio.h | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
Index: v2.6.24-rc4/sound/pci/echoaudio/echoaudio.c
===================================================================
--- v2.6.24-rc4.orig/sound/pci/echoaudio/echoaudio.c
+++ v2.6.24-rc4/sound/pci/echoaudio/echoaudio.c
@@ -378,7 +378,7 @@ static int pcm_digital_in_open(struct sn
DE_ACT(("pcm_digital_in_open\n"));
max_channels = num_digital_busses_in(chip) - substream->number;
- down(&chip->mode_mutex);
+ mutex_lock(&chip->mode_mutex);
if (chip->digital_mode == DIGITAL_MODE_ADAT)
err = pcm_open(substream, max_channels);
else /* If the card has ADAT, subtract the 6 channels
@@ -405,7 +405,7 @@ static int pcm_digital_in_open(struct sn
chip->can_set_rate=0;
din_exit:
- up(&chip->mode_mutex);
+ mutex_unlock(&chip->mode_mutex);
return err;
}
@@ -420,7 +420,7 @@ static int pcm_digital_out_open(struct s
DE_ACT(("pcm_digital_out_open\n"));
max_channels = num_digital_busses_out(chip) - substream->number;
- down(&chip->mode_mutex);
+ mutex_lock(&chip->mode_mutex);
if (chip->digital_mode == DIGITAL_MODE_ADAT)
err = pcm_open(substream, max_channels);
else /* If the card has ADAT, subtract the 6 channels
@@ -447,7 +447,7 @@ static int pcm_digital_out_open(struct s
if (atomic_read(&chip->opencount) > 1 && chip->rate_set)
chip->can_set_rate=0;
dout_exit:
- up(&chip->mode_mutex);
+ mutex_unlock(&chip->mode_mutex);
return err;
}
@@ -1420,7 +1420,7 @@ static int snd_echo_digital_mode_put(str
if (dmode != chip->digital_mode) {
/* mode_mutex is required to make this operation atomic wrt
pcm_digital_*_open() and set_input_clock() functions. */
- down(&chip->mode_mutex);
+ mutex_lock(&chip->mode_mutex);
/* Do not allow the user to change the digital mode when a pcm
device is open because it also changes the number of channels
@@ -1439,7 +1439,7 @@ static int snd_echo_digital_mode_put(str
if (changed >= 0)
changed = 1; /* No errors */
}
- up(&chip->mode_mutex);
+ mutex_unlock(&chip->mode_mutex);
}
return changed;
}
@@ -1566,12 +1566,12 @@ static int snd_echo_clock_source_put(str
return -EINVAL;
dclock = chip->clock_source_list[eclock];
if (chip->input_clock != dclock) {
- down(&chip->mode_mutex);
+ mutex_lock(&chip->mode_mutex);
spin_lock_irq(&chip->lock);
if ((changed = set_input_clock(chip, dclock)) == 0)
changed = 1; /* no errors */
spin_unlock_irq(&chip->lock);
- up(&chip->mode_mutex);
+ mutex_unlock(&chip->mode_mutex);
}
if (changed < 0)
@@ -1972,7 +1972,7 @@ static __devinit int snd_echo_create(str
return err;
}
atomic_set(&chip->opencount, 0);
- init_MUTEX(&chip->mode_mutex);
+ mutex_init(&chip->mode_mutex);
chip->can_set_rate = 1;
*rchip = chip;
/* Init done ! */
Index: v2.6.24-rc4/sound/pci/echoaudio/echoaudio.h
===================================================================
--- v2.6.24-rc4.orig/sound/pci/echoaudio/echoaudio.h
+++ v2.6.24-rc4/sound/pci/echoaudio/echoaudio.h
@@ -361,7 +361,7 @@ struct echoaudio {
spinlock_t lock;
struct snd_pcm_substream *substream[DSP_MAXPIPES];
int last_period[DSP_MAXPIPES];
- struct semaphore mode_mutex;
+ struct mutex mode_mutex;
u16 num_digital_modes, digital_mode_list[6];
u16 num_clock_sources, clock_source_list[10];
atomic_t opencount;
--
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch 2/2] 9p util semaphore to mutex
[not found] <20071209171509.601451827@gmail.com>
2007-12-09 17:15 ` [patch 1/2] echoaudio semaphore to mutex Kevin Winchester
@ 2007-12-09 17:15 ` Kevin Winchester
2007-12-12 2:06 ` Andrew Morton
1 sibling, 1 reply; 5+ messages in thread
From: Kevin Winchester @ 2007-12-09 17:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Ingo Molnar, Eric Van Hensbergen, Ron Minnich, Latchesar Ionkov,
v9fs-developer, linux-kernel, Kevin Winchester
[-- Attachment #1: 9p-util-sem-to-mutex.patch --]
[-- Type: text/plain, Size: 1537 bytes --]
Convert the semaphore to a mutex in net/9p/util.c
Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
---
net/9p/util.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Index: v2.6.24-rc4/net/9p/util.c
===================================================================
--- v2.6.24-rc4.orig/net/9p/util.c
+++ v2.6.24-rc4/net/9p/util.c
@@ -33,7 +33,7 @@
#include <net/9p/9p.h>
struct p9_idpool {
- struct semaphore lock;
+ struct mutex lock;
struct idr pool;
};
@@ -45,7 +45,7 @@ struct p9_idpool *p9_idpool_create(void)
if (!p)
return ERR_PTR(-ENOMEM);
- init_MUTEX(&p->lock);
+ mutex_init(&p->lock);
idr_init(&p->pool);
return p;
@@ -76,14 +76,14 @@ retry:
if (idr_pre_get(&p->pool, GFP_KERNEL) == 0)
return 0;
- if (down_interruptible(&p->lock) == -EINTR) {
+ if (mutex_lock_interruptible(&p->lock) == -EINTR) {
P9_EPRINTK(KERN_WARNING, "Interrupted while locking\n");
return -1;
}
/* no need to store exactly p, we just need something non-null */
error = idr_get_new(&p->pool, p, &i);
- up(&p->lock);
+ mutex_unlock(&p->lock);
if (error == -EAGAIN)
goto retry;
@@ -104,12 +104,12 @@ EXPORT_SYMBOL(p9_idpool_get);
void p9_idpool_put(int id, struct p9_idpool *p)
{
- if (down_interruptible(&p->lock) == -EINTR) {
+ if (mutex_lock_interruptible(&p->lock) == -EINTR) {
P9_EPRINTK(KERN_WARNING, "Interrupted while locking\n");
return;
}
idr_remove(&p->pool, id);
- up(&p->lock);
+ mutex_unlock(&p->lock);
}
EXPORT_SYMBOL(p9_idpool_put);
--
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 1/2] echoaudio semaphore to mutex
2007-12-09 17:15 ` [patch 1/2] echoaudio semaphore to mutex Kevin Winchester
@ 2007-12-10 9:06 ` Giuliano Pochini
2007-12-17 10:15 ` Takashi Iwai
1 sibling, 0 replies; 5+ messages in thread
From: Giuliano Pochini @ 2007-12-10 9:06 UTC (permalink / raw)
To: Kevin Winchester
Cc: Andrew Morton, Ingo Molnar, Jaroslav Kysela, linux-kernel
On Sun, 9 Dec 2007, Kevin Winchester wrote:
> Convert the semaphore to a mutex in echoaudio.c
>
> Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
It's an 1:1 change. It looks fine to me.
Acked-by: Giuliano Pochini <pochini@shiny.it>
--
Giuliano.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 2/2] 9p util semaphore to mutex
2007-12-09 17:15 ` [patch 2/2] 9p util " Kevin Winchester
@ 2007-12-12 2:06 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2007-12-12 2:06 UTC (permalink / raw)
To: Kevin Winchester
Cc: Ingo Molnar, Eric Van Hensbergen, Ron Minnich, Latchesar Ionkov,
v9fs-developer, linux-kernel
On Sun, 09 Dec 2007 13:15:11 -0400 Kevin Winchester <kjwinchester@gmail.com> wrote:
> Convert the semaphore to a mutex in net/9p/util.c
>
> Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
>
> ---
> net/9p/util.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> Index: v2.6.24-rc4/net/9p/util.c
> ===================================================================
> --- v2.6.24-rc4.orig/net/9p/util.c
> +++ v2.6.24-rc4/net/9p/util.c
> @@ -33,7 +33,7 @@
> #include <net/9p/9p.h>
>
> struct p9_idpool {
> - struct semaphore lock;
> + struct mutex lock;
> struct idr pool;
> };
>
> @@ -45,7 +45,7 @@ struct p9_idpool *p9_idpool_create(void)
> if (!p)
> return ERR_PTR(-ENOMEM);
>
> - init_MUTEX(&p->lock);
> + mutex_init(&p->lock);
> idr_init(&p->pool);
>
> return p;
> @@ -76,14 +76,14 @@ retry:
> if (idr_pre_get(&p->pool, GFP_KERNEL) == 0)
> return 0;
>
> - if (down_interruptible(&p->lock) == -EINTR) {
> + if (mutex_lock_interruptible(&p->lock) == -EINTR) {
> P9_EPRINTK(KERN_WARNING, "Interrupted while locking\n");
> return -1;
> }
>
> /* no need to store exactly p, we just need something non-null */
> error = idr_get_new(&p->pool, p, &i);
> - up(&p->lock);
> + mutex_unlock(&p->lock);
>
> if (error == -EAGAIN)
> goto retry;
> @@ -104,12 +104,12 @@ EXPORT_SYMBOL(p9_idpool_get);
>
> void p9_idpool_put(int id, struct p9_idpool *p)
> {
> - if (down_interruptible(&p->lock) == -EINTR) {
> + if (mutex_lock_interruptible(&p->lock) == -EINTR) {
> P9_EPRINTK(KERN_WARNING, "Interrupted while locking\n");
> return;
> }
> idr_remove(&p->pool, id);
> - up(&p->lock);
> + mutex_unlock(&p->lock);
> }
> EXPORT_SYMBOL(p9_idpool_put);
I cannot see how the code which you are modifying could have been correct.
If the lock is contended and this task has signal_pending() then boom - we
return from p9_idpool_put() without having removed the item from the IDR
tree and then we just lose all track of this fact. It is at a minimum a
memory leak.
I'm getting the feeling that we need to go and take a general look at the
down_interuptible() and mutex_lock_interruptible() callers in the nether
regions of the kernel...
Meanwhile I'd propose this instead:
From: Andrew Morton <akpm@linux-foundation.org>
Don't use down_interruptible() in situations where we cannot handle the
consequences.
Cc: Kevin Winchester <kjwinchester@gmail.com>
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Cc: Ron Minnich <rminnich@sandia.gov>
Cc: Latchesar Ionkov <lucho@ionkov.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
net/9p/util.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff -puN net/9p/util.c~9p-util-fix-semaphore-handling net/9p/util.c
--- a/net/9p/util.c~9p-util-fix-semaphore-handling
+++ a/net/9p/util.c
@@ -76,12 +76,8 @@ retry:
if (idr_pre_get(&p->pool, GFP_KERNEL) == 0)
return 0;
- if (down_interruptible(&p->lock) == -EINTR) {
- P9_EPRINTK(KERN_WARNING, "Interrupted while locking\n");
- return -1;
- }
-
/* no need to store exactly p, we just need something non-null */
+ down(&p->lock);
error = idr_get_new(&p->pool, p, &i);
up(&p->lock);
@@ -104,10 +100,7 @@ EXPORT_SYMBOL(p9_idpool_get);
void p9_idpool_put(int id, struct p9_idpool *p)
{
- if (down_interruptible(&p->lock) == -EINTR) {
- P9_EPRINTK(KERN_WARNING, "Interrupted while locking\n");
- return;
- }
+ down(&p->lock);
idr_remove(&p->pool, id);
up(&p->lock);
}
_
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 1/2] echoaudio semaphore to mutex
2007-12-09 17:15 ` [patch 1/2] echoaudio semaphore to mutex Kevin Winchester
2007-12-10 9:06 ` Giuliano Pochini
@ 2007-12-17 10:15 ` Takashi Iwai
1 sibling, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2007-12-17 10:15 UTC (permalink / raw)
To: Kevin Winchester
Cc: Andrew Morton, Ingo Molnar, Giuliano Pochini, Jaroslav Kysela,
linux-kernel
At Sun, 09 Dec 2007 13:15:10 -0400,
Kevin Winchester wrote:
>
> Convert the semaphore to a mutex in echoaudio.c
>
> Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
It was already fixed on ALSA tree :)
Thanks anyway.
Takashi
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-12-17 11:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20071209171509.601451827@gmail.com>
2007-12-09 17:15 ` [patch 1/2] echoaudio semaphore to mutex Kevin Winchester
2007-12-10 9:06 ` Giuliano Pochini
2007-12-17 10:15 ` Takashi Iwai
2007-12-09 17:15 ` [patch 2/2] 9p util " Kevin Winchester
2007-12-12 2:06 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).