[PATCH 0/5] use mutex instead of semaphore in several drivers

[PATCH 0/5] use mutex instead of semaphore in several drivers

[Matthias Kaehlcke]

This patchset converts semaphores that are used as mutexes to the
mutex API in the following drivers/code:

Host AP driver
OnStream SCSI Tape driver
SCSI Tape driver
ISDN subsystem common functions
DVB frontend tuning interface

-- 
Matthias Kaehlcke
Linux Application Developer
Barcelona

        Representation of the world, like the world itself, is
        the work of men; they describe it from their own point
         of view, which they  confuse with the absolute truth
                          (Simone de Beauvoir)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

[PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Matthias Kaehlcke]

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                  `-

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Michael Buesch]

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.

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Satyam Sharma]

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

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Satyam Sharma]

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

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Matthias Kaehlcke]

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                  `-

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Satyam Sharma]

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>

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[John W. Linville]

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

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Satyam Sharma]

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

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Randy Dunlap]

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 ***

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Michael Buesch]

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

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Andrew Morton]

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.

Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver

[Satyam Sharma]

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

[PATCH 2/5] Use mutex instead of semaphore in the OnStream SCSI Tape driver

[Matthias Kaehlcke]

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                  `-

Re: [PATCH 2/5] Use mutex instead of semaphore in the OnStream SCSI Tape driver

[Satyam Sharma]

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>

[PATCH 3/5] Use mutex instead of semaphore in the SCSI Tape driver

[Matthias Kaehlcke]

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                  `-

Re: [PATCH 3/5] Use mutex instead of semaphore in the SCSI Tape driver

[Satyam Sharma]

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>

Re: [PATCH 3/5] Use mutex instead of semaphore in the SCSI Tape driver

[Kai Makisara]

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

[PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions

[Matthias Kaehlcke]

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                  `-

Re: [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions

[Satyam Sharma]

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>

Re: [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions

[Karsten Keil]

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)

Re: [PATCH 4/5] Use mutex instead of semaphore in ISDN subsystem common functions

[Andrew Morton]

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.

[PATCH 5/5] Use mutex instead of semaphore in the DVB frontend tuning interface

[Matthias Kaehlcke]

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                  `-

Re: [PATCH 5/5] Use mutex instead of semaphore in the DVB frontend tuning interface

[Satyam Sharma]

On Sun, 29 Jul 2007, Matthias Kaehlcke wrote:

> 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>

Reviewed-by: Satyam Sharma <satyam@infradead.org>