* [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
* 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 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
* [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 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
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).