* [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-29 21:29 [PATCH 0/5] use mutex instead of semaphore in several drivers Matthias Kaehlcke
@ 2007-07-29 21:34 ` Matthias Kaehlcke
2007-07-29 22:06 ` Michael Buesch
2007-07-29 21:36 ` [PATCH 2/5] Use mutex instead of semaphore in the OnStream SCSI Tape driver Matthias Kaehlcke
` (3 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Matthias Kaehlcke @ 2007-07-29 21:34 UTC (permalink / raw)
To: linville, linux-wireless; +Cc: linux-kernel, akpm
The Host AP driver uses a semaphore as mutex. Use the mutex API
instead of the (binary) semaphore.
Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
--
- res = down_interruptible(&local->rid_bap_sem);
+ res = mutex_lock_interruptible(&local->rid_bap_mtx);
if (res)
return res;
@@ -834,7 +834,7 @@ static int hfa384x_get_rid(struct net_device *dev, u16 rid, void *buf, int len,
printk(KERN_DEBUG "%s: hfa384x_get_rid: CMDCODE_ACCESS failed "
"(res=%d, rid=%04x, len=%d)\n",
dev->name, res, rid, len);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
return res;
}
@@ -861,7 +861,7 @@ static int hfa384x_get_rid(struct net_device *dev, u16 rid, void *buf, int len,
res = hfa384x_from_bap(dev, BAP0, buf, len);
spin_unlock_bh(&local->baplock);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
if (res) {
if (res != -ENODATA)
@@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
/* RID len in words and +1 for rec.rid */
rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
- res = down_interruptible(&local->rid_bap_sem);
+ res = mutex_lock_interruptible(&local->rid_bap_mtx);
if (res)
return res;
@@ -917,12 +917,12 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
if (res) {
printk(KERN_DEBUG "%s: hfa384x_set_rid (rid=%04x, len=%d) - "
"failed - res=%d\n", dev->name, rid, len, res);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
return res;
}
res = hfa384x_cmd(dev, HFA384X_CMDCODE_ACCESS_WRITE, rid, NULL, NULL);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
if (res) {
printk(KERN_DEBUG "%s: hfa384x_set_rid: CMDCODE_ACCESS_WRITE "
@@ -3171,7 +3171,7 @@ prism2_init_local_data(struct prism2_helper_functions *funcs, int card_idx,
spin_lock_init(&local->cmdlock);
spin_lock_init(&local->baplock);
spin_lock_init(&local->lock);
- init_MUTEX(&local->rid_bap_sem);
+ mutex_init(&local->rid_bap_mtx);
if (card_idx < 0 || card_idx >= MAX_PARM_DEVICES)
card_idx = 0;
diff --git a/drivers/net/wireless/hostap/hostap_wlan.h b/drivers/net/wireless/hostap/hostap_wlan.h
index 87a54aa..a42325c 100644
--- a/drivers/net/wireless/hostap/hostap_wlan.h
+++ b/drivers/net/wireless/hostap/hostap_wlan.h
@@ -3,6 +3,7 @@
#include <linux/wireless.h>
#include <linux/netdevice.h>
+#include <linux/mutex.h>
#include <net/iw_handler.h>
#include "hostap_config.h"
@@ -641,7 +642,7 @@ struct local_info {
* when removing entries from the list.
* TX and RX paths can use read lock. */
spinlock_t cmdlock, baplock, lock;
- struct semaphore rid_bap_sem;
+ struct mutex rid_bap_mtx;
u16 infofid; /* MAC buffer id for info frame */
/* txfid, intransmitfid, next_txtid, and next_alloc are protected by
* txfidlock */
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
Si deseas mantener tu libertad, debes estar preparado para defenderla
(Richard Stallman)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-29 21:34 ` [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver Matthias Kaehlcke
@ 2007-07-29 22:06 ` Michael Buesch
2007-07-30 3:18 ` Satyam Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Michael Buesch @ 2007-07-29 22:06 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: linville, linux-wireless, linux-kernel, akpm
On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:
> The Host AP driver uses a semaphore as mutex. Use the mutex API
> instead of the (binary) semaphore.
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
>
> --
>
> - res = down_interruptible(&local->rid_bap_sem);
> + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> if (res)
> return res;
>
> @@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
> /* RID len in words and +1 for rec.rid */
> rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
>
> - res = down_interruptible(&local->rid_bap_sem);
> + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> if (res)
> return res;
>
Is res returned to userspace? If yes, that's not right.
On a interrupted mutex allocation you should return
-ERESTARTSYS to userspace.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-29 22:06 ` Michael Buesch
@ 2007-07-30 3:18 ` Satyam Sharma
2007-07-30 3:47 ` Satyam Sharma
2007-07-30 17:09 ` Michael Buesch
0 siblings, 2 replies; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 3:18 UTC (permalink / raw)
To: Michael Buesch
Cc: Matthias Kaehlcke, linville, linux-wireless, linux-kernel, akpm
On Mon, 30 Jul 2007, Michael Buesch wrote:
> On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:
> > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > instead of the (binary) semaphore.
> >
> > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
[ Something seems to have gone wrong with your diff / patch / script.
There was no diff header here, which should have been. ]
> > - res = down_interruptible(&local->rid_bap_sem);
> > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > if (res)
> > return res;
> >
> > @@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
> > /* RID len in words and +1 for rec.rid */
> > rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
> >
> > - res = down_interruptible(&local->rid_bap_sem);
> > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > if (res)
> > return res;
> >
>
> Is res returned to userspace? If yes, that's not right.
Yup, that's not right.
> On a interrupted mutex allocation you should return
> -ERESTARTSYS to userspace.
Nope, userspace must not see ERESTARTSYS (I /think/ this is the third
time I'm participating in this exact same discussion :-)
If the return would be caught by a previous in-kernel caller in the
call chain, ERESTARTSYS is okay and it could try to restart the
operation. However, if the return goes unfiltered directly to
userspace, EINTR is the correct choice.
Satyam
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 3:18 ` Satyam Sharma
@ 2007-07-30 3:47 ` Satyam Sharma
2007-07-30 5:40 ` Matthias Kaehlcke
2007-07-30 17:09 ` Michael Buesch
1 sibling, 1 reply; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 3:47 UTC (permalink / raw)
To: Michael Buesch
Cc: Matthias Kaehlcke, linville, linux-wireless, linux-kernel, akpm
Whoops ...
On Mon, 30 Jul 2007, Satyam Sharma wrote:
> On Mon, 30 Jul 2007, Michael Buesch wrote:
>
> > On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:
> > > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > > instead of the (binary) semaphore.
> > >
> > > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
>
> [ Something seems to have gone wrong with your diff / patch / script.
> There was no diff header here, which should have been. ]
>
> > > - res = down_interruptible(&local->rid_bap_sem);
> > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > if (res)
> > > return res;
> > >
> > > @@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
> > > /* RID len in words and +1 for rec.rid */
> > > rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
> > >
> > > - res = down_interruptible(&local->rid_bap_sem);
> > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > if (res)
> > > return res;
> > >
> >
> > Is res returned to userspace? If yes, that's not right.
>
> Yup, that's not right.
>
> > On a interrupted mutex allocation you should return
> > -ERESTARTSYS to userspace.
>
> Nope, userspace must not see ERESTARTSYS (I /think/ this is the third
> time I'm participating in this exact same discussion :-)
>
> If the return would be caught by a previous in-kernel caller in the
> call chain, ERESTARTSYS is okay and it could try to restart the
> operation. However, if the return goes unfiltered directly to
> userspace, EINTR is the correct choice.
Eek, and because mutex_lock_interruptible() *does* return -EINTR when
interrupted by a signal, and I noticed that hfa384x_set_rid() could be
called from an ioctl(2) codepath with no intermediate caller trying to
restart it, so the code is perfectly alright, actually.
But the patch doesn't have the diff header anyway, so Matthias would
probably have to resend in any case :-)
Satyam
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 3:47 ` Satyam Sharma
@ 2007-07-30 5:40 ` Matthias Kaehlcke
2007-07-30 7:20 ` Satyam Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Matthias Kaehlcke @ 2007-07-30 5:40 UTC (permalink / raw)
To: Satyam Sharma
Cc: Michael Buesch, linville, linux-wireless, linux-kernel, akpm
El Mon, Jul 30, 2007 at 09:17:25AM +0530 Satyam Sharma ha dit:
> Whoops ...
>
>
> On Mon, 30 Jul 2007, Satyam Sharma wrote:
>
> > On Mon, 30 Jul 2007, Michael Buesch wrote:
> >
> > > On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:
> > > > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > > > instead of the (binary) semaphore.
> > > >
> > > > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
> >
> > [ Something seems to have gone wrong with your diff / patch / script.
> > There was no diff header here, which should have been. ]
> >
> > > > - res = down_interruptible(&local->rid_bap_sem);
> > > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > > if (res)
> > > > return res;
> > > >
> > > > @@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
> > > > /* RID len in words and +1 for rec.rid */
> > > > rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
> > > >
> > > > - res = down_interruptible(&local->rid_bap_sem);
> > > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > > if (res)
> > > > return res;
> > > >
> > >
> > > Is res returned to userspace? If yes, that's not right.
> >
> > Yup, that's not right.
> >
> > > On a interrupted mutex allocation you should return
> > > -ERESTARTSYS to userspace.
> >
> > Nope, userspace must not see ERESTARTSYS (I /think/ this is the third
> > time I'm participating in this exact same discussion :-)
> >
> > If the return would be caught by a previous in-kernel caller in the
> > call chain, ERESTARTSYS is okay and it could try to restart the
> > operation. However, if the return goes unfiltered directly to
> > userspace, EINTR is the correct choice.
>
> Eek, and because mutex_lock_interruptible() *does* return -EINTR when
> interrupted by a signal, and I noticed that hfa384x_set_rid() could be
> called from an ioctl(2) codepath with no intermediate caller trying to
> restart it, so the code is perfectly alright, actually.
>
> But the patch doesn't have the diff header anyway, so Matthias would
> probably have to resend in any case :-)
thanks for reviewing the patch and for your comments, here is the
patch with the diff header (no idea what when wrong with the other
one)
regards
matthias
--
The Host AP driver uses a semaphore as mutex. Use the mutex API
instead of the (binary) semaphore.
Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
--
diff --git a/drivers/net/wireless/hostap/hostap_hw.c b/drivers/net/wireless/hostap/hostap_hw.c
index 959887b..d3dacbc 100644
--- a/drivers/net/wireless/hostap/hostap_hw.c
+++ b/drivers/net/wireless/hostap/hostap_hw.c
@@ -825,7 +825,7 @@ static int hfa384x_get_rid(struct net_device *dev, u16 rid, void *buf, int len,
local->hw_downloading)
return -ENODEV;
- res = down_interruptible(&local->rid_bap_sem);
+ res = mutex_lock_interruptible(&local->rid_bap_mtx);
if (res)
return res;
@@ -834,7 +834,7 @@ static int hfa384x_get_rid(struct net_device *dev, u16 rid, void *buf, int len,
printk(KERN_DEBUG "%s: hfa384x_get_rid: CMDCODE_ACCESS failed "
"(res=%d, rid=%04x, len=%d)\n",
dev->name, res, rid, len);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
return res;
}
@@ -861,7 +861,7 @@ static int hfa384x_get_rid(struct net_device *dev, u16 rid, void *buf, int len,
res = hfa384x_from_bap(dev, BAP0, buf, len);
spin_unlock_bh(&local->baplock);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
if (res) {
if (res != -ENODATA)
@@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
/* RID len in words and +1 for rec.rid */
rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
- res = down_interruptible(&local->rid_bap_sem);
+ res = mutex_lock_interruptible(&local->rid_bap_mtx);
if (res)
return res;
@@ -917,12 +917,12 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
if (res) {
printk(KERN_DEBUG "%s: hfa384x_set_rid (rid=%04x, len=%d) - "
"failed - res=%d\n", dev->name, rid, len, res);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
return res;
}
res = hfa384x_cmd(dev, HFA384X_CMDCODE_ACCESS_WRITE, rid, NULL, NULL);
- up(&local->rid_bap_sem);
+ mutex_unlock(&local->rid_bap_mtx);
if (res) {
printk(KERN_DEBUG "%s: hfa384x_set_rid: CMDCODE_ACCESS_WRITE "
@@ -3171,7 +3171,7 @@ prism2_init_local_data(struct prism2_helper_functions *funcs, int card_idx,
spin_lock_init(&local->cmdlock);
spin_lock_init(&local->baplock);
spin_lock_init(&local->lock);
- init_MUTEX(&local->rid_bap_sem);
+ mutex_init(&local->rid_bap_mtx);
if (card_idx < 0 || card_idx >= MAX_PARM_DEVICES)
card_idx = 0;
diff --git a/drivers/net/wireless/hostap/hostap_wlan.h b/drivers/net/wireless/hostap/hostap_wlan.h
index 87a54aa..a42325c 100644
--- a/drivers/net/wireless/hostap/hostap_wlan.h
+++ b/drivers/net/wireless/hostap/hostap_wlan.h
@@ -3,6 +3,7 @@
#include <linux/wireless.h>
#include <linux/netdevice.h>
+#include <linux/mutex.h>
#include <net/iw_handler.h>
#include "hostap_config.h"
@@ -641,7 +642,7 @@ struct local_info {
* when removing entries from the list.
* TX and RX paths can use read lock. */
spinlock_t cmdlock, baplock, lock;
- struct semaphore rid_bap_sem;
+ struct mutex rid_bap_mtx;
u16 infofid; /* MAC buffer id for info frame */
/* txfid, intransmitfid, next_txtid, and next_alloc are protected by
* txfidlock */
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
You must have a plan. If you don't have a plan,
you'll become part of somebody else's plan
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 5:40 ` Matthias Kaehlcke
@ 2007-07-30 7:20 ` Satyam Sharma
2007-07-30 14:50 ` John W. Linville
0 siblings, 1 reply; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 7:20 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Michael Buesch, linville, linux-wireless, linux-kernel, akpm
On Mon, 30 Jul 2007, Matthias Kaehlcke wrote:
> [...]
>
> The Host AP driver uses a semaphore as mutex. Use the mutex API
> instead of the (binary) semaphore.
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
Reviewed-by: Satyam Sharma <satyam@infradead.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 7:20 ` Satyam Sharma
@ 2007-07-30 14:50 ` John W. Linville
2007-07-30 20:14 ` Satyam Sharma
0 siblings, 1 reply; 28+ messages in thread
From: John W. Linville @ 2007-07-30 14:50 UTC (permalink / raw)
To: Satyam Sharma
Cc: Matthias Kaehlcke, Michael Buesch, linux-wireless, linux-kernel,
akpm
On Mon, Jul 30, 2007 at 12:50:31PM +0530, Satyam Sharma wrote:
>
>
> On Mon, 30 Jul 2007, Matthias Kaehlcke wrote:
>
> > [...]
> >
> > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > instead of the (binary) semaphore.
> >
> > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
>
> Reviewed-by: Satyam Sharma <satyam@infradead.org>
Is that the same as Acked-by? Or do you disagree with the patch?
Just checkin'...
John
--
John W. Linville
linville@tuxdriver.com
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 14:50 ` John W. Linville
@ 2007-07-30 20:14 ` Satyam Sharma
2007-07-30 20:13 ` Randy Dunlap
0 siblings, 1 reply; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 20:14 UTC (permalink / raw)
To: John W. Linville
Cc: Matthias Kaehlcke, Michael Buesch, linux-wireless, linux-kernel,
akpm
On Mon, 30 Jul 2007, John W. Linville wrote:
> On Mon, Jul 30, 2007 at 12:50:31PM +0530, Satyam Sharma wrote:
> > On Mon, 30 Jul 2007, Matthias Kaehlcke wrote:
> >
> > > [...]
> > > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > > instead of the (binary) semaphore.
> > >
> > > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
> >
> > Reviewed-by: Satyam Sharma <satyam@infradead.org>
>
> Is that the same as Acked-by? Or do you disagree with the patch?
>
> Just checkin'...
Well, I'm not the maintainer, so didn't think correct to give an
"Acked-by". "Reviewed-by" generally means I read the patch, and think
it is correct. Probably could save time for others ...
Satyam
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 20:14 ` Satyam Sharma
@ 2007-07-30 20:13 ` Randy Dunlap
0 siblings, 0 replies; 28+ messages in thread
From: Randy Dunlap @ 2007-07-30 20:13 UTC (permalink / raw)
To: Satyam Sharma
Cc: John W. Linville, Matthias Kaehlcke, Michael Buesch,
linux-wireless, linux-kernel, akpm
On Tue, 31 Jul 2007 01:44:37 +0530 (IST) Satyam Sharma wrote:
>
>
> On Mon, 30 Jul 2007, John W. Linville wrote:
>
> > On Mon, Jul 30, 2007 at 12:50:31PM +0530, Satyam Sharma wrote:
> > > On Mon, 30 Jul 2007, Matthias Kaehlcke wrote:
> > >
> > > > [...]
> > > > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > > > instead of the (binary) semaphore.
> > > >
> > > > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
> > >
> > > Reviewed-by: Satyam Sharma <satyam@infradead.org>
> >
> > Is that the same as Acked-by? Or do you disagree with the patch?
> >
> > Just checkin'...
>
> Well, I'm not the maintainer, so didn't think correct to give an
> "Acked-by". "Reviewed-by" generally means I read the patch, and think
> it is correct. Probably could save time for others ...
You should use Acked-by. See Documentation/SubmittingPatches. :)
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 3:18 ` Satyam Sharma
2007-07-30 3:47 ` Satyam Sharma
@ 2007-07-30 17:09 ` Michael Buesch
2007-07-30 19:23 ` Andrew Morton
1 sibling, 1 reply; 28+ messages in thread
From: Michael Buesch @ 2007-07-30 17:09 UTC (permalink / raw)
To: Satyam Sharma
Cc: Matthias Kaehlcke, linville, linux-wireless, linux-kernel, akpm
On Monday 30 July 2007, Satyam Sharma wrote:
>
> On Mon, 30 Jul 2007, Michael Buesch wrote:
>
> > On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:
> > > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > > instead of the (binary) semaphore.
> > >
> > > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
>
> [ Something seems to have gone wrong with your diff / patch / script.
> There was no diff header here, which should have been. ]
>
> > > - res = down_interruptible(&local->rid_bap_sem);
> > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > if (res)
> > > return res;
> > >
> > > @@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
> > > /* RID len in words and +1 for rec.rid */
> > > rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
> > >
> > > - res = down_interruptible(&local->rid_bap_sem);
> > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > if (res)
> > > return res;
> > >
> >
> > Is res returned to userspace? If yes, that's not right.
>
> Yup, that's not right.
>
> > On a interrupted mutex allocation you should return
> > -ERESTARTSYS to userspace.
>
> Nope, userspace must not see ERESTARTSYS (I /think/ this is the third
> time I'm participating in this exact same discussion :-)
>
> If the return would be caught by a previous in-kernel caller in the
> call chain, ERESTARTSYS is okay and it could try to restart the
> operation. However, if the return goes unfiltered directly to
> userspace, EINTR is the correct choice.
Last time I submitted a patch which returned EINTR to userspace,
people came and said it was wrong. It was said that we must
return ERESTARTSYS to the libc and the libc will convert that
into either EINTR or automatically restart it immediately.
(You can set that in the sigaction stuff).
So either the one who told me that last time was wrong, or you. :)
Andrew? I think you were involved in this discussion I mentioned.
(It was about the HW-RNG chardev, but that doesn't matter here).
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 17:09 ` Michael Buesch
@ 2007-07-30 19:23 ` Andrew Morton
2007-07-30 20:11 ` Satyam Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Morton @ 2007-07-30 19:23 UTC (permalink / raw)
To: Michael Buesch
Cc: Satyam Sharma, Matthias Kaehlcke, linville, linux-wireless,
linux-kernel
On Mon, 30 Jul 2007 19:09:38 +0200
Michael Buesch <mb@bu3sch.de> wrote:
> On Monday 30 July 2007, Satyam Sharma wrote:
> >
> > On Mon, 30 Jul 2007, Michael Buesch wrote:
> >
> > > On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:
> > > > The Host AP driver uses a semaphore as mutex. Use the mutex API
> > > > instead of the (binary) semaphore.
> > > >
> > > > Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
> >
> > [ Something seems to have gone wrong with your diff / patch / script.
> > There was no diff header here, which should have been. ]
> >
> > > > - res = down_interruptible(&local->rid_bap_sem);
> > > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > > if (res)
> > > > return res;
> > > >
> > > > @@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
> > > > /* RID len in words and +1 for rec.rid */
> > > > rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
> > > >
> > > > - res = down_interruptible(&local->rid_bap_sem);
> > > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > > if (res)
> > > > return res;
> > > >
> > >
> > > Is res returned to userspace? If yes, that's not right.
> >
> > Yup, that's not right.
> >
> > > On a interrupted mutex allocation you should return
> > > -ERESTARTSYS to userspace.
> >
> > Nope, userspace must not see ERESTARTSYS (I /think/ this is the third
> > time I'm participating in this exact same discussion :-)
> >
> > If the return would be caught by a previous in-kernel caller in the
> > call chain, ERESTARTSYS is okay and it could try to restart the
> > operation. However, if the return goes unfiltered directly to
> > userspace, EINTR is the correct choice.
>
> Last time I submitted a patch which returned EINTR to userspace,
> people came and said it was wrong. It was said that we must
> return ERESTARTSYS to the libc and the libc will convert that
> into either EINTR or automatically restart it immediately.
> (You can set that in the sigaction stuff).
>
> So either the one who told me that last time was wrong, or you. :)
>
> Andrew? I think you were involved in this discussion I mentioned.
> (It was about the HW-RNG chardev, but that doesn't matter here).
who, me? signals? Danger.
yes, I think ERESTARTSYS is correct. That gets converted to -EINTR by the
arch's signal handling code (not by glibc) just before we return to
userspace.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
2007-07-30 19:23 ` Andrew Morton
@ 2007-07-30 20:11 ` Satyam Sharma
0 siblings, 0 replies; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 20:11 UTC (permalink / raw)
To: Andrew Morton
Cc: Michael Buesch, Matthias Kaehlcke, linville, linux-wireless,
linux-kernel
On Mon, 30 Jul 2007, Andrew Morton wrote:
> On Mon, 30 Jul 2007 19:09:38 +0200
> Michael Buesch <mb@bu3sch.de> wrote:
>
> > On Monday 30 July 2007, Satyam Sharma wrote:
> > >
> > > On Mon, 30 Jul 2007, Michael Buesch wrote:
> > >
> > > > On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:
> > > > >
> > > > > @@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len)
> > > > > /* RID len in words and +1 for rec.rid */
> > > > > rec.len = cpu_to_le16(len / 2 + len % 2 + 1);
> > > > >
> > > > > - res = down_interruptible(&local->rid_bap_sem);
> > > > > + res = mutex_lock_interruptible(&local->rid_bap_mtx);
> > > > > if (res)
> > > > > return res;
> > > > >
> > > >
> > > > Is res returned to userspace? If yes, that's not right.
> > >
> > > Yup, that's not right.
> > >
> > > > On a interrupted mutex allocation you should return
> > > > -ERESTARTSYS to userspace.
> > >
> > > Nope, userspace must not see ERESTARTSYS (I /think/ this is the third
> > > time I'm participating in this exact same discussion :-)
> > >
> > > If the return would be caught by a previous in-kernel caller in the
> > > call chain, ERESTARTSYS is okay and it could try to restart the
> > > operation. However, if the return goes unfiltered directly to
> > > userspace, EINTR is the correct choice.
> >
> > Last time I submitted a patch which returned EINTR to userspace,
> > people came and said it was wrong. It was said that we must
> > return ERESTARTSYS to the libc and the libc will convert that
> > into either EINTR or automatically restart it immediately.
> > (You can set that in the sigaction stuff).
> >
> > So either the one who told me that last time was wrong, or you. :)
> >
> > Andrew? I think you were involved in this discussion I mentioned.
> > (It was about the HW-RNG chardev, but that doesn't matter here).
>
> who, me? signals? Danger.
>
> yes, I think ERESTARTSYS is correct. That gets converted to -EINTR by the
> arch's signal handling code (not by glibc) just before we return to
> userspace.
Hmm? But the question is whether which of EINTR or ERESTARTSYS is correct
to be returned to userspace (so, EINTR).
But now that you pointed to the automagic ERESTARTSYS-to-EINTR conversion
in the arch/ code, I think there's no problem returning ERESTARTSYS from
syscalls in generic kernel code when interrupted by a signal either (if
all arch's do that conversion).
Probably makes sense to change down_interruptible() and
mutex_lock_interruptible() to return ERESTARTSYS (and not EINTR) in
that case. wait_event_interruptible() is already ERESTARTSYS, I see ...
Satyam
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/5] Use mutex instead of semaphore in the OnStream SCSI Tape driver
2007-07-29 21:29 [PATCH 0/5] use mutex instead of semaphore in several drivers Matthias Kaehlcke
2007-07-29 21:34 ` [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver Matthias Kaehlcke
@ 2007-07-29 21:36 ` Matthias Kaehlcke
2007-07-30 3:27 ` Satyam Sharma
2007-07-29 21:38 ` [PATCH 3/5] Use mutex instead of semaphore in the " Matthias Kaehlcke
` (2 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Matthias Kaehlcke @ 2007-07-29 21:36 UTC (permalink / raw)
To: osst, osst-users, linux-scsi; +Cc: linux-kernel, akpm
The OnStream SCSI Tape driver uses a semaphore as mutex. Use the mutex
API instead of the (binary) semaphore.
Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
--
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 08060fb..0e2452c 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -3298,7 +3298,7 @@ static ssize_t osst_write(struct file * filp, const char __user * buf, size_t co
char * name = tape_name(STp);
- if (down_interruptible(&STp->lock))
+ if (mutex_lock_interruptible(&STp->lock))
return (-ERESTARTSYS);
/*
@@ -3600,7 +3600,7 @@ if (SRpnt) printk(KERN_ERR "%s:A: Not supposed to have SRpnt at line %d\n", name
out:
if (SRpnt != NULL) osst_release_request(SRpnt);
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
return retval;
}
@@ -3619,7 +3619,7 @@ static ssize_t osst_read(struct file * filp, char __user * buf, size_t count, lo
char * name = tape_name(STp);
- if (down_interruptible(&STp->lock))
+ if (mutex_lock_interruptible(&STp->lock))
return (-ERESTARTSYS);
/*
@@ -3785,7 +3785,7 @@ static ssize_t osst_read(struct file * filp, char __user * buf, size_t count, lo
out:
if (SRpnt != NULL) osst_release_request(SRpnt);
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
return retval;
}
@@ -4852,7 +4852,7 @@ static int osst_ioctl(struct inode * inode,struct file * file,
char * name = tape_name(STp);
void __user * p = (void __user *)arg;
- if (down_interruptible(&STp->lock))
+ if (mutex_lock_interruptible(&STp->lock))
return -ERESTARTSYS;
#if DEBUG
@@ -5163,14 +5163,14 @@ static int osst_ioctl(struct inode * inode,struct file * file,
}
if (SRpnt) osst_release_request(SRpnt);
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
return scsi_ioctl(STp->device, cmd_in, p);
out:
if (SRpnt) osst_release_request(SRpnt);
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
return retval;
}
@@ -5866,7 +5866,7 @@ static int osst_probe(struct device *dev)
tpnt->modes[2].defined = 1;
tpnt->density_changed = tpnt->compression_changed = tpnt->blksize_changed = 0;
- init_MUTEX(&tpnt->lock);
+ mutex_init(&tpnt->lock);
osst_nr_dev++;
write_unlock(&os_scsi_tapes_lock);
diff --git a/drivers/scsi/osst.h b/drivers/scsi/osst.h
index 2cc7b5a..5aa2274 100644
--- a/drivers/scsi/osst.h
+++ b/drivers/scsi/osst.h
@@ -4,6 +4,7 @@
#include <asm/byteorder.h>
#include <linux/completion.h>
+#include <linux/mutex.h>
/* FIXME - rename and use the following two types or delete them!
* and the types really should go to st.h anyway...
@@ -532,7 +533,7 @@ struct osst_tape {
struct scsi_driver *driver;
unsigned capacity;
struct scsi_device *device;
- struct semaphore lock; /* for serialization */
+ struct mutex lock; /* for serialization */
struct completion wait; /* for SCSI commands */
struct osst_buffer * buffer;
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
Don't walk behind me, I may not lead
Don't walk in front of me, I may not follow
Just walk beside me and be my friend
(Albert Camus)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 2/5] Use mutex instead of semaphore in the OnStream SCSI Tape driver
2007-07-29 21:36 ` [PATCH 2/5] Use mutex instead of semaphore in the OnStream SCSI Tape driver Matthias Kaehlcke
@ 2007-07-30 3:27 ` Satyam Sharma
0 siblings, 0 replies; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 3:27 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: osst, osst-users, linux-scsi, linux-kernel, akpm
Hi,
On Sun, 29 Jul 2007, Matthias Kaehlcke wrote:
> The OnStream SCSI Tape driver uses a semaphore as mutex. Use the mutex
> API instead of the (binary) semaphore.
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
> [...]
> @@ -3298,7 +3298,7 @@ static ssize_t osst_write(struct file * filp, const char __user * buf, size_t co
> char * name = tape_name(STp);
>
>
> - if (down_interruptible(&STp->lock))
> + if (mutex_lock_interruptible(&STp->lock))
> return (-ERESTARTSYS);
The () after return existed in the code already, but you could've felt
free to remove them :-)
> @@ -3619,7 +3619,7 @@ static ssize_t osst_read(struct file * filp, char __user * buf, size_t count, lo
> char * name = tape_name(STp);
>
>
> - if (down_interruptible(&STp->lock))
> + if (mutex_lock_interruptible(&STp->lock))
> return (-ERESTARTSYS);
Same here.
Reviewed-by: Satyam Sharma <satyam@infradead.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] Use mutex instead of semaphore in the SCSI Tape driver
2007-07-29 21:29 [PATCH 0/5] use mutex instead of semaphore in several drivers Matthias Kaehlcke
2007-07-29 21:34 ` [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver Matthias Kaehlcke
2007-07-29 21:36 ` [PATCH 2/5] Use mutex instead of semaphore in the OnStream SCSI Tape driver Matthias Kaehlcke
@ 2007-07-29 21:38 ` Matthias Kaehlcke
2007-07-30 3:29 ` Satyam Sharma
2007-07-30 16:27 ` Kai Makisara
2007-07-29 21:41 ` [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions Matthias Kaehlcke
2007-07-29 21:43 ` [PATCH 5/5] Use mutex instead of semaphore in the DVB frontend tuning interface Matthias Kaehlcke
4 siblings, 2 replies; 28+ messages in thread
From: Matthias Kaehlcke @ 2007-07-29 21:38 UTC (permalink / raw)
To: Kai.Makisara, linux-scsi; +Cc: linux-kernel, akpm
The SCSI Tape driver uses a semaphore as mutex. Use the mutex API
instead of the (binary) semaphore.
Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
--
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index a4f7b84..73c44cb 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -1485,7 +1485,7 @@ st_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
struct st_buffer *STbp;
char *name = tape_name(STp);
- if (down_interruptible(&STp->lock))
+ if (mutex_lock_interruptible(&STp->lock))
return -ERESTARTSYS;
retval = rw_checks(STp, filp, count);
@@ -1736,7 +1736,7 @@ st_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
if (SRpnt != NULL)
st_release_request(SRpnt);
release_buffering(STp, 0);
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
return retval;
}
@@ -1942,7 +1942,7 @@ st_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
struct st_buffer *STbp = STp->buffer;
DEB( char *name = tape_name(STp); )
- if (down_interruptible(&STp->lock))
+ if (mutex_lock_interruptible(&STp->lock))
return -ERESTARTSYS;
retval = rw_checks(STp, filp, count);
@@ -2069,7 +2069,7 @@ st_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
release_buffering(STp, 1);
STbp->buffer_bytes = 0;
}
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
return retval;
}
@@ -3226,7 +3226,7 @@ static int st_ioctl(struct inode *inode, struct file *file,
char *name = tape_name(STp);
void __user *p = (void __user *)arg;
- if (down_interruptible(&STp->lock))
+ if (mutex_lock_interruptible(&STp->lock))
return -ERESTARTSYS;
DEB(
@@ -3537,7 +3537,7 @@ static int st_ioctl(struct inode *inode, struct file *file,
retval = (-EFAULT);
goto out;
}
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
switch (cmd_in) {
case SCSI_IOCTL_GET_IDLUN:
case SCSI_IOCTL_GET_BUS_NUMBER:
@@ -3563,7 +3563,7 @@ static int st_ioctl(struct inode *inode, struct file *file,
return retval;
out:
- up(&STp->lock);
+ mutex_unlock(&STp->lock);
return retval;
}
@@ -4029,7 +4029,7 @@ static int st_probe(struct device *dev)
tpnt->density_changed = tpnt->compression_changed =
tpnt->blksize_changed = 0;
- init_MUTEX(&tpnt->lock);
+ mutex_init(&tpnt->lock);
st_nr_dev++;
write_unlock(&st_dev_arr_lock);
diff --git a/drivers/scsi/st.h b/drivers/scsi/st.h
index 50f3deb..6c80757 100644
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -3,6 +3,7 @@
#define _ST_H
#include <linux/completion.h>
+#include <linux/mutex.h>
#include <linux/kref.h>
#include <scsi/scsi_cmnd.h>
@@ -98,7 +99,7 @@ struct st_partstat {
struct scsi_tape {
struct scsi_driver *driver;
struct scsi_device *device;
- struct semaphore lock; /* For serialization */
+ struct mutex lock; /* For serialization */
struct completion wait; /* For SCSI commands */
struct st_buffer *buffer;
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
Ma patrie est où je suis, où personne ne me dérange, où personne
ne me demande que je suis, d'où je viens et ce que je fais
(B. Traven)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 3/5] Use mutex instead of semaphore in the SCSI Tape driver
2007-07-29 21:38 ` [PATCH 3/5] Use mutex instead of semaphore in the " Matthias Kaehlcke
@ 2007-07-30 3:29 ` Satyam Sharma
2007-07-30 16:27 ` Kai Makisara
1 sibling, 0 replies; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 3:29 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: Kai.Makisara, linux-scsi, linux-kernel, akpm
On Sun, 29 Jul 2007, Matthias Kaehlcke wrote:
> The SCSI Tape driver uses a semaphore as mutex. Use the mutex API
> instead of the (binary) semaphore.
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
Reviewed-by: Satyam Sharma <satyam@infradead.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] Use mutex instead of semaphore in the SCSI Tape driver
2007-07-29 21:38 ` [PATCH 3/5] Use mutex instead of semaphore in the " Matthias Kaehlcke
2007-07-30 3:29 ` Satyam Sharma
@ 2007-07-30 16:27 ` Kai Makisara
1 sibling, 0 replies; 28+ messages in thread
From: Kai Makisara @ 2007-07-30 16:27 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: linux-scsi, linux-kernel, akpm
On Sun, 29 Jul 2007, Matthias Kaehlcke wrote:
> The SCSI Tape driver uses a semaphore as mutex. Use the mutex API
> instead of the (binary) semaphore.
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
>
Signed-off-by: Kai Makisara <kai.makisara@kolumbus.fi>
Thanks.
--
Kai
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions
2007-07-29 21:29 [PATCH 0/5] use mutex instead of semaphore in several drivers Matthias Kaehlcke
` (2 preceding siblings ...)
2007-07-29 21:38 ` [PATCH 3/5] Use mutex instead of semaphore in the " Matthias Kaehlcke
@ 2007-07-29 21:41 ` Matthias Kaehlcke
2007-07-30 3:51 ` Satyam Sharma
` (2 more replies)
2007-07-29 21:43 ` [PATCH 5/5] Use mutex instead of semaphore in the DVB frontend tuning interface Matthias Kaehlcke
4 siblings, 3 replies; 28+ messages in thread
From: Matthias Kaehlcke @ 2007-07-29 21:41 UTC (permalink / raw)
To: kkeil, kai.germaschewski, isdn4linux; +Cc: linux-kernel, akpm
The ISDN subsystem common functions use a semaphore as mutex. Use the
mutex API instead of the (binary) semaphore.
Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
--
diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c
index c97330b..a5d3db4 100644
--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -1364,7 +1364,7 @@ isdn_ioctl(struct inode *inode, struct file *file, uint cmd, ulong arg)
} else {
s = NULL;
}
- ret = down_interruptible(&dev->sem);
+ ret = mutex_lock_interruptible(&dev->mtx);
if( ret ) return ret;
if ((s = isdn_net_new(s, NULL))) {
if (copy_to_user(argp, s, strlen(s) + 1)){
@@ -1374,7 +1374,7 @@ isdn_ioctl(struct inode *inode, struct file *file, uint cmd, ulong arg)
}
} else
ret = -ENODEV;
- up(&dev->sem);
+ mutex_unlock(&dev->mtx);
return ret;
case IIOCNETASL:
/* Add a slave to a network-interface */
@@ -1383,7 +1383,7 @@ isdn_ioctl(struct inode *inode, struct file *file, uint cmd, ulong arg)
return -EFAULT;
} else
return -EINVAL;
- ret = down_interruptible(&dev->sem);
+ ret = mutex_lock_interruptible(&dev->mtx);
if( ret ) return ret;
if ((s = isdn_net_newslave(bname))) {
if (copy_to_user(argp, s, strlen(s) + 1)){
@@ -1393,17 +1393,17 @@ isdn_ioctl(struct inode *inode, struct file *file, uint cmd, ulong arg)
}
} else
ret = -ENODEV;
- up(&dev->sem);
+ mutex_unlock(&dev->mtx);
return ret;
case IIOCNETDIF:
/* Delete a network-interface */
if (arg) {
if (copy_from_user(name, argp, sizeof(name)))
return -EFAULT;
- ret = down_interruptible(&dev->sem);
+ ret = mutex_lock_interruptible(&dev->mtx);
if( ret ) return ret;
ret = isdn_net_rm(name);
- up(&dev->sem);
+ mutex_unlock(&dev->mtx);
return ret;
} else
return -EINVAL;
@@ -1432,10 +1432,10 @@ isdn_ioctl(struct inode *inode, struct file *file, uint cmd, ulong arg)
if (arg) {
if (copy_from_user(&phone, argp, sizeof(phone)))
return -EFAULT;
- ret = down_interruptible(&dev->sem);
+ ret = mutex_lock_interruptible(&dev->mtx);
if( ret ) return ret;
ret = isdn_net_addphone(&phone);
- up(&dev->sem);
+ mutex_unlock(&dev->mtx);
return ret;
} else
return -EINVAL;
@@ -1444,10 +1444,10 @@ isdn_ioctl(struct inode *inode, struct file *file, uint cmd, ulong arg)
if (arg) {
if (copy_from_user(&phone, argp, sizeof(phone)))
return -EFAULT;
- ret = down_interruptible(&dev->sem);
+ ret = mutex_lock_interruptible(&dev->mtx);
if( ret ) return ret;
ret = isdn_net_getphones(&phone, argp);
- up(&dev->sem);
+ mutex_unlock(&dev->mtx);
return ret;
} else
return -EINVAL;
@@ -1456,10 +1456,10 @@ isdn_ioctl(struct inode *inode, struct file *file, uint cmd, ulong arg)
if (arg) {
if (copy_from_user(&phone, argp, sizeof(phone)))
return -EFAULT;
- ret = down_interruptible(&dev->sem);
+ ret = mutex_lock_interruptible(&dev->mtx);
if( ret ) return ret;
ret = isdn_net_delphone(&phone);
- up(&dev->sem);
+ mutex_unlock(&dev->mtx);
return ret;
} else
return -EINVAL;
@@ -2303,7 +2303,7 @@ static int __init isdn_init(void)
#ifdef MODULE
dev->owner = THIS_MODULE;
#endif
- init_MUTEX(&dev->sem);
+ mutex_init(&dev->mtx);
init_waitqueue_head(&dev->info_waitq);
for (i = 0; i < ISDN_MAX_CHANNELS; i++) {
dev->drvmap[i] = -1;
diff --git a/include/linux/isdn.h b/include/linux/isdn.h
index 3c7875b..5a77203 100644
--- a/include/linux/isdn.h
+++ b/include/linux/isdn.h
@@ -15,6 +15,7 @@
#define __ISDN_H__
#include <linux/ioctl.h>
+#include <linux/mutex.h>
#ifdef CONFIG_COBALT_MICRO_SERVER
/* Save memory */
@@ -624,7 +625,7 @@ typedef struct isdn_devt {
int v110emu[ISDN_MAX_CHANNELS]; /* V.110 emulator-mode 0=none */
atomic_t v110use[ISDN_MAX_CHANNELS]; /* Usage-Semaphore for stream */
isdn_v110_stream *v110[ISDN_MAX_CHANNELS]; /* V.110 private data */
- struct semaphore sem; /* serialize list access*/
+ struct mutex mtx; /* serialize list access*/
unsigned long global_features;
} isdn_dev;
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
Tant qu'il y aura sur terre des hommes pour qui existe un concept
d' 'honneur national', la menace d'une nouvelle guerre subsistera.
(B. Traven)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions
2007-07-29 21:41 ` [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions Matthias Kaehlcke
@ 2007-07-30 3:51 ` Satyam Sharma
2007-07-30 14:27 ` Karsten Keil
2007-07-31 8:57 ` Andrew Morton
2 siblings, 0 replies; 28+ messages in thread
From: Satyam Sharma @ 2007-07-30 3:51 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: kkeil, kai.germaschewski, isdn4linux, linux-kernel, akpm
On Sun, 29 Jul 2007, Matthias Kaehlcke wrote:
> The ISDN subsystem common functions use a semaphore as mutex. Use the
> mutex API instead of the (binary) semaphore.
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
Reviewed-by: Satyam Sharma <satyam@infradead.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions
2007-07-29 21:41 ` [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions Matthias Kaehlcke
2007-07-30 3:51 ` Satyam Sharma
@ 2007-07-30 14:27 ` Karsten Keil
2007-07-31 8:57 ` Andrew Morton
2 siblings, 0 replies; 28+ messages in thread
From: Karsten Keil @ 2007-07-30 14:27 UTC (permalink / raw)
To: Matthias Kaehlcke, kkeil, kai.germaschewski, isdn4linux,
linux-kernel, akpm
On Sun, Jul 29, 2007 at 11:41:05PM +0200, Matthias Kaehlcke wrote:
> The ISDN subsystem common functions use a semaphore as mutex. Use the
> mutex API instead of the (binary) semaphore.
>
> Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
>
Acked-by: Karsten Keil <kkeil@suse.de>
--
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 16746 (AG Nuernberg)
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions
2007-07-29 21:41 ` [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions Matthias Kaehlcke
2007-07-30 3:51 ` Satyam Sharma
2007-07-30 14:27 ` Karsten Keil
@ 2007-07-31 8:57 ` Andrew Morton
2 siblings, 0 replies; 28+ messages in thread
From: Andrew Morton @ 2007-07-31 8:57 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: kkeil, kai.germaschewski, isdn4linux, linux-kernel
On Sun, 29 Jul 2007 23:41:05 +0200 Matthias Kaehlcke <matthias.kaehlcke@gmail.com> wrote:
> --- a/include/linux/isdn.h
> +++ b/include/linux/isdn.h
> @@ -15,6 +15,7 @@
> #define __ISDN_H__
>
> #include <linux/ioctl.h>
> +#include <linux/mutex.h>
Breaks `make headers_check'.
That was covered in sections 2 and 20 of Documentation/SubmitChecklist,
which nobody reads.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/5] Use mutex instead of semaphore in the DVB frontend tuning interface
2007-07-29 21:29 [PATCH 0/5] use mutex instead of semaphore in several drivers Matthias Kaehlcke
` (3 preceding siblings ...)
2007-07-29 21:41 ` [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions Matthias Kaehlcke
@ 2007-07-29 21:43 ` Matthias Kaehlcke
2007-07-30 3:53 ` Satyam Sharma
4 siblings, 1 reply; 28+ messages in thread
From: Matthias Kaehlcke @ 2007-07-29 21:43 UTC (permalink / raw)
To: v4l-dvb-maintainer; +Cc: linux-kernel, akpm
The DVB frontend tuning interface uses a semaphore as mutex. Use the
mutex API instead of the (binary) semaphore.
Signed-off-by: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
--
diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index b6c7f66..e35dd17 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -138,7 +138,7 @@ static void dvb_frontend_add_event(struct dvb_frontend *fe, fe_status_t status)
dprintk ("%s\n", __FUNCTION__);
- if (down_interruptible (&events->sem))
+ if (mutex_lock_interruptible (&events->mtx))
return;
wp = (events->eventw + 1) % MAX_EVENT;
@@ -159,7 +159,7 @@ static void dvb_frontend_add_event(struct dvb_frontend *fe, fe_status_t status)
events->eventw = wp;
- up (&events->sem);
+ mutex_unlock(&events->mtx);
e->status = status;
@@ -197,7 +197,7 @@ static int dvb_frontend_get_event(struct dvb_frontend *fe,
return ret;
}
- if (down_interruptible (&events->sem))
+ if (mutex_lock_interruptible (&events->mtx))
return -ERESTARTSYS;
memcpy (event, &events->events[events->eventr],
@@ -205,7 +205,7 @@ static int dvb_frontend_get_event(struct dvb_frontend *fe,
events->eventr = (events->eventr + 1) % MAX_EVENT;
- up (&events->sem);
+ mutex_unlock(&events->mtx);
return 0;
}
@@ -1080,7 +1080,7 @@ int dvb_register_frontend(struct dvb_adapter* dvb,
init_MUTEX (&fepriv->sem);
init_waitqueue_head (&fepriv->wait_queue);
init_waitqueue_head (&fepriv->events.wait_queue);
- init_MUTEX (&fepriv->events.sem);
+ mutex_init(&fepriv->events.mtx);
fe->dvb = dvb;
fepriv->inversion = INVERSION_OFF;
diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.h b/drivers/media/dvb/dvb-core/dvb_frontend.h
index a770a87..f95de63 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.h
@@ -35,6 +35,7 @@
#include <linux/module.h>
#include <linux/errno.h>
#include <linux/delay.h>
+#include <linux/mutex.h>
#include <linux/dvb/frontend.h>
@@ -142,7 +143,7 @@ struct dvb_fe_events {
int eventr;
int overflow;
wait_queue_head_t wait_queue;
- struct semaphore sem;
+ struct mutex mtx;
};
struct dvb_frontend {
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
El camino se hace al andar
(Antonio Machado)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply related [flat|nested] 28+ messages in thread