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

[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

[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

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

[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