Skip to contentSkip to conversation

Use mutex insted of semaphore

PATCH In review Ksenija Stanojevic 3 participants 7 messages Oct 2, 2015 – Oct 3, 2015 Staging

lore
Share & tools
lore

Ksenija Stanojevic

author
#0 lore raw

v2 cover Use mutex insted of semaphore

This patchset replaces binary semaphore with mutex in comedi drivers.

Signed-off-by Ksenija Stanojevic <[email protected]>

Ksenija Stanojevic (4):
Staging: comedi: Use mutex instead of semaphore
Staging: comedi: Use mutex instead of semaphore
Staging: comedi: Use mutex instead of semaphore
Staging: comedi: Use mutex instead of semaphore

 drivers/staging/comedi/drivers/ni_usb6501.c  | 16 +++----
 drivers/staging/comedi/drivers/usbdux.c      | 64 ++++++++++++-------------
 drivers/staging/comedi/drivers/usbduxfast.c  | 36 +++++++-------
 drivers/staging/comedi/drivers/usbduxsigma.c | 70 ++++++++++++++--------------
 4 files changed, 93 insertions(+), 93 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v2 1/4 Staging: comedi: Use mutex instead of semaphore in usbduxsigma.c

Replace binary semaphore with mutex because mutex gives better performance.
This change is safe because the thread that decrements the value of semaphore is also the one that increments it, and acts like a mutex where owner of the lock is the only one that can release the lock.

Signed-off-by Ksenija Stanojevic <[email protected]>

---
Changes in v2:
-change subject line and commit message

 drivers/staging/comedi/drivers/usbduxsigma.c | 70 ++++++++++++++--------------
 1 file changed, 35 insertions(+), 35 deletions(-)
drivers/staging/comedi/drivers/usbduxsigma.c +35 -35
index d49147b..456e9f1 100644
@@ -173,7 +173,7 @@ struct usbduxsigma_private {
 	unsigned int ai_interval;
 	/* commands */
 	u8 *dux_commands;
-	struct semaphore sem;
+	struct mutex mut;
 };
 
 static void usbduxsigma_unlink_urbs(struct urb **urbs, int num_urbs)

@@ -199,10 +199,10 @@ static int usbduxsigma_ai_cancel(struct comedi_device *dev,
 {
 	struct usbduxsigma_private *devpriv = dev->private;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if it is really running */
 	usbduxsigma_ai_stop(dev, devpriv->ai_cmd_running);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -325,10 +325,10 @@ static int usbduxsigma_ao_cancel(struct comedi_device *dev,
 {
 	struct usbduxsigma_private *devpriv = dev->private;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if it is really running */
 	usbduxsigma_ao_stop(dev, devpriv->ao_cmd_running);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -609,19 +609,19 @@ static int usbduxsigma_ai_inttrig(struct comedi_device *dev,
 	if (trig_num != cmd->start_arg)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	if (!devpriv->ai_cmd_running) {
 		devpriv->ai_cmd_running = 1;
 		ret = usbduxsigma_submit_urbs(dev, devpriv->ai_urbs,
 					      devpriv->n_ai_urbs, 1);
 		if (ret < 0) {
 			devpriv->ai_cmd_running = 0;
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 		s->async->inttrig = NULL;
 	}
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 1;
 }

@@ -638,7 +638,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
 	int ret;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->high_speed) {
 		/*

@@ -673,7 +673,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
 
 	ret = usbbuxsigma_send_cmd(dev, USBBUXSIGMA_AD_CMD);
 	if (ret < 0) {
-		up(&devpriv->sem);
+		mutex_unlock(&devpriv->mut);
 		return ret;
 	}
 

@@ -686,7 +686,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
 					      devpriv->n_ai_urbs, 1);
 		if (ret < 0) {
 			devpriv->ai_cmd_running = 0;
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 		s->async->inttrig = NULL;

@@ -694,7 +694,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
 		s->async->inttrig = usbduxsigma_ai_inttrig;
 	}
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -712,9 +712,9 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,
 	int ret;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	if (devpriv->ai_cmd_running) {
-		up(&devpriv->sem);
+		mutex_unlock(&devpriv->mut);
 		return -EBUSY;
 	}
 

@@ -731,7 +731,7 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,
 	/* adc commands */
 	ret = usbbuxsigma_send_cmd(dev, USBDUXSIGMA_SINGLE_AD_CMD);
 	if (ret < 0) {
-		up(&devpriv->sem);
+		mutex_unlock(&devpriv->mut);
 		return ret;
 	}
 

@@ -740,7 +740,7 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,
 
 		ret = usbduxsigma_receive_cmd(dev, USBDUXSIGMA_SINGLE_AD_CMD);
 		if (ret < 0) {
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 

@@ -750,7 +750,7 @@ static int usbduxsigma_ai_insn_read(struct comedi_device *dev,
 		val &= 0x00ffffff;	/* strip status byte */
 		data[i] = comedi_offset_munge(s, val);
 	}
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return insn->n;
 }

@@ -763,9 +763,9 @@ static int usbduxsigma_ao_insn_read(struct comedi_device *dev,
 	struct usbduxsigma_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	ret = comedi_readback_insn_read(dev, s, insn, data);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -780,9 +780,9 @@ static int usbduxsigma_ao_insn_write(struct comedi_device *dev,
 	int ret;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	if (devpriv->ao_cmd_running) {
-		up(&devpriv->sem);
+		mutex_unlock(&devpriv->mut);
 		return -EBUSY;
 	}
 

@@ -792,12 +792,12 @@ static int usbduxsigma_ao_insn_write(struct comedi_device *dev,
 		devpriv->dux_commands[3] = chan;	/* channel number */
 		ret = usbbuxsigma_send_cmd(dev, USBDUXSIGMA_DA_CMD);
 		if (ret < 0) {
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 		s->readback[chan] = data[i];
 	}
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return insn->n;
 }

@@ -813,19 +813,19 @@ static int usbduxsigma_ao_inttrig(struct comedi_device *dev,
 	if (trig_num != cmd->start_arg)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	if (!devpriv->ao_cmd_running) {
 		devpriv->ao_cmd_running = 1;
 		ret = usbduxsigma_submit_urbs(dev, devpriv->ao_urbs,
 					      devpriv->n_ao_urbs, 0);
 		if (ret < 0) {
 			devpriv->ao_cmd_running = 0;
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 		s->async->inttrig = NULL;
 	}
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 1;
 }

@@ -856,7 +856,7 @@ static int usbduxsigma_ao_cmdtest(struct comedi_device *dev,
 	err |= comedi_check_trigger_src(&cmd->stop_src, TRIG_COUNT | TRIG_NONE);
 
 	if (err) {
-		up(&devpriv->sem);
+		mutex_unlock(&devpriv->mut);
 		return 1;
 	}
 

@@ -905,7 +905,7 @@ static int usbduxsigma_ao_cmd(struct comedi_device *dev,
 	struct comedi_cmd *cmd = &s->async->cmd;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	/*
 	 * For now, only "scan" timing is supported.  A future version may

@@ -924,7 +924,7 @@ static int usbduxsigma_ao_cmd(struct comedi_device *dev,
 					      devpriv->n_ao_urbs, 0);
 		if (ret < 0) {
 			devpriv->ao_cmd_running = 0;
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 		s->async->inttrig = NULL;

@@ -932,7 +932,7 @@ static int usbduxsigma_ao_cmd(struct comedi_device *dev,
 		s->async->inttrig = usbduxsigma_ao_inttrig;
 	}
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -963,7 +963,7 @@ static int usbduxsigma_dio_insn_bits(struct comedi_device *dev,
 	struct usbduxsigma_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	comedi_dio_update_state(s, data);
 

@@ -990,7 +990,7 @@ static int usbduxsigma_dio_insn_bits(struct comedi_device *dev,
 	ret = insn->n;
 
 done:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -1462,7 +1462,7 @@ static int usbduxsigma_auto_attach(struct comedi_device *dev,
 	if (!devpriv)
 		return -ENOMEM;
 
-	sema_init(&devpriv->sem, 1);
+	mutex_init(&devpriv->mut);
 
 	usb_set_intfdata(intf, devpriv);
 

@@ -1576,7 +1576,7 @@ static void usbduxsigma_detach(struct comedi_device *dev)
 	if (!devpriv)
 		return;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	/* force unlink all urbs */
 	usbduxsigma_ai_stop(dev, 1);

@@ -1585,7 +1585,7 @@ static void usbduxsigma_detach(struct comedi_device *dev)
 
 	usbduxsigma_free_usb_buffers(dev);
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 }
 
 static struct comedi_driver usbduxsigma_driver = {
Signature · 1 line
1.9.1
Reviewed-by on v2 1/4
#2 lore raw

Re: [Outreachy kernel] [PATCH v2 1/4] Staging: comedi: Use mutex instead of semaphore in usbduxsigma.c

Quoted 8 lines · On Friday 02 October 2015 22:04:22 Ksenija Stanojevic wrote:

Replace binary semaphore with mutex because mutex gives better performance.
This change is safe because the thread that decrements the value of semaphore is also the one that increments it, and acts like a mutex where owner of the lock is the only one that can release the lock.

Signed-off-by Ksenija Stanojevic <[email protected]>

(all four patches)

Reviewed-by Arnd Bergmann <[email protected]>

Generally speaking however, the subject line is nicer if you put the driver name in front, to make it easier to distinguish the subject lines, like:

staging: comedi/usbduxsigma: use mutex instead of semaphore

Greg will let you know if he wants you to respin the patches once more or if he just takes them as they are.

Arnd

author
#3 lore raw

PATCH v2 2/4 Staging: comedi: Use mutex instead of semaphore in usbdux.c

In reply to Ksenija Stanojevic

Replace binary semaphore with mutex because mutex gives better performance.
This change is safe because the thread that decrements the value of semaphore is also the one that increments it, and acts like a mutex where owner of the lock is the only one that can release the lock.

Signed-off-by Ksenija Stanojevic <[email protected]>

---
Changes in v2:
-change subject line and commit message

 drivers/staging/comedi/drivers/usbdux.c | 64 ++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 32 deletions(-)
drivers/staging/comedi/drivers/usbdux.c +32 -32
index 39710f2..f4f05d2 100644
@@ -210,7 +210,7 @@ struct usbdux_private {
 	unsigned int ai_interval;
 	/* commands */
 	u8 *dux_commands;
-	struct semaphore sem;
+	struct mutex mut;
 };
 
 static void usbdux_unlink_urbs(struct urb **urbs, int num_urbs)

@@ -237,10 +237,10 @@ static int usbdux_ai_cancel(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 
 	/* prevent other CPUs from submitting new commands just now */
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if the urb really has been submitted */
 	usbdux_ai_stop(dev, devpriv->ai_cmd_running);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -365,10 +365,10 @@ static int usbdux_ao_cancel(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 
 	/* prevent other CPUs from submitting a command just now */
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if it is really running */
 	usbdux_ao_stop(dev, devpriv->ao_cmd_running);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -646,7 +646,7 @@ static int usbdux_ai_inttrig(struct comedi_device *dev,
 	if (trig_num != cmd->start_arg)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (!devpriv->ai_cmd_running) {
 		devpriv->ai_cmd_running = 1;

@@ -662,7 +662,7 @@ static int usbdux_ai_inttrig(struct comedi_device *dev,
 	}
 
 ai_trig_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 	return ret;
 }
 

@@ -675,7 +675,7 @@ static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	int i;
 
 	/* block other CPUs from starting an ai_cmd */
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ai_cmd_running)
 		goto ai_cmd_exit;

@@ -736,7 +736,7 @@ static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	}
 
 ai_cmd_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -754,7 +754,7 @@ static int usbdux_ai_insn_read(struct comedi_device *dev,
 	int ret = -EBUSY;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ai_cmd_running)
 		goto ai_read_exit;

@@ -782,7 +782,7 @@ static int usbdux_ai_insn_read(struct comedi_device *dev,
 	}
 
 ai_read_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -795,9 +795,9 @@ static int usbdux_ao_insn_read(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	ret = comedi_readback_insn_read(dev, s, insn, data);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -814,7 +814,7 @@ static int usbdux_ao_insn_write(struct comedi_device *dev,
 	int ret = -EBUSY;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ao_cmd_running)
 		goto ao_write_exit;

@@ -838,7 +838,7 @@ static int usbdux_ao_insn_write(struct comedi_device *dev,
 	}
 
 ao_write_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -854,7 +854,7 @@ static int usbdux_ao_inttrig(struct comedi_device *dev,
 	if (trig_num != cmd->start_arg)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (!devpriv->ao_cmd_running) {
 		devpriv->ao_cmd_running = 1;

@@ -870,7 +870,7 @@ static int usbdux_ao_inttrig(struct comedi_device *dev,
 	}
 
 ao_trig_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 	return ret;
 }
 

@@ -960,7 +960,7 @@ static int usbdux_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	struct comedi_cmd *cmd = &s->async->cmd;
 	int ret = -EBUSY;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ao_cmd_running)
 		goto ao_cmd_exit;

@@ -1002,7 +1002,7 @@ static int usbdux_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	}
 
 ao_cmd_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -1033,7 +1033,7 @@ static int usbdux_dio_insn_bits(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	comedi_dio_update_state(s, data);
 

@@ -1055,7 +1055,7 @@ static int usbdux_dio_insn_bits(struct comedi_device *dev,
 	data[1] = le16_to_cpu(devpriv->insn_buf[1]);
 
 dio_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -1070,7 +1070,7 @@ static int usbdux_counter_read(struct comedi_device *dev,
 	int ret = 0;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	for (i = 0; i < insn->n; i++) {
 		ret = send_dux_commands(dev, USBDUX_CMD_TIMER_RD);

@@ -1084,7 +1084,7 @@ static int usbdux_counter_read(struct comedi_device *dev,
 	}
 
 counter_read_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -1100,7 +1100,7 @@ static int usbdux_counter_write(struct comedi_device *dev,
 	int ret = 0;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	devpriv->dux_commands[1] = chan;
 

@@ -1112,7 +1112,7 @@ static int usbdux_counter_write(struct comedi_device *dev,
 			break;
 	}
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -1148,11 +1148,11 @@ static int usbdux_pwm_cancel(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if it is really running */
 	usbdux_pwm_stop(dev, devpriv->pwm_cmd_running);
 	ret = send_dux_commands(dev, USBDUX_CMD_PWM_OFF);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -1257,7 +1257,7 @@ static int usbdux_pwm_start(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret = 0;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->pwm_cmd_running)
 		goto pwm_start_exit;

@@ -1276,7 +1276,7 @@ static int usbdux_pwm_start(struct comedi_device *dev,
 		devpriv->pwm_cmd_running = 0;
 
 pwm_start_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -1576,7 +1576,7 @@ static int usbdux_auto_attach(struct comedi_device *dev,
 	if (!devpriv)
 		return -ENOMEM;
 
-	sema_init(&devpriv->sem, 1);
+	mutex_init(&devpriv->mut);
 
 	usb_set_intfdata(intf, devpriv);
 

@@ -1691,7 +1691,7 @@ static void usbdux_detach(struct comedi_device *dev)
 	if (!devpriv)
 		return;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	/* force unlink all urbs */
 	usbdux_pwm_stop(dev, 1);

@@ -1700,7 +1700,7 @@ static void usbdux_detach(struct comedi_device *dev)
 
 	usbdux_free_usb_buffers(dev);
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 }
 
 static struct comedi_driver usbdux_driver = {
Signature · 1 line
1.9.1
#4 lore raw

Re: [Outreachy kernel] [PATCH v2 2/4] Staging: comedi: Use mutex instead of semaphore in usbdux.c

Quoted 5 lines · Ksenija Stanojevic

Replace binary semaphore with mutex because mutex gives better performance.
This change is safe because the thread that decrements the value of semaphore is also the one that increments it, and acts like a mutex where owner of the lock is the only one that can release the lock.

Very nice commit message.

julia

Quoted 290 lines
Signed-off-by Ksenija Stanojevic <[email protected]>

---
Changes in v2:
-change subject line and commit message

 drivers/staging/comedi/drivers/usbdux.c | 64 ++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 32 deletions(-)
drivers/staging/comedi/drivers/usbdux.c +32 -32
index 39710f2..f4f05d2 100644
@@ -210,7 +210,7 @@ struct usbdux_private {
 	unsigned int ai_interval;
 	/* commands */
 	u8 *dux_commands;
-	struct semaphore sem;
+	struct mutex mut;
 };
 
 static void usbdux_unlink_urbs(struct urb **urbs, int num_urbs)

@@ -237,10 +237,10 @@ static int usbdux_ai_cancel(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 
 	/* prevent other CPUs from submitting new commands just now */
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if the urb really has been submitted */
 	usbdux_ai_stop(dev, devpriv->ai_cmd_running);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -365,10 +365,10 @@ static int usbdux_ao_cancel(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 
 	/* prevent other CPUs from submitting a command just now */
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if it is really running */
 	usbdux_ao_stop(dev, devpriv->ao_cmd_running);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return 0;
 }

@@ -646,7 +646,7 @@ static int usbdux_ai_inttrig(struct comedi_device *dev,
 	if (trig_num != cmd->start_arg)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (!devpriv->ai_cmd_running) {
 		devpriv->ai_cmd_running = 1;

@@ -662,7 +662,7 @@ static int usbdux_ai_inttrig(struct comedi_device *dev,
 	}
 
 ai_trig_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 	return ret;
 }
 

@@ -675,7 +675,7 @@ static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	int i;
 
 	/* block other CPUs from starting an ai_cmd */
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ai_cmd_running)
 		goto ai_cmd_exit;

@@ -736,7 +736,7 @@ static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	}
 
 ai_cmd_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -754,7 +754,7 @@ static int usbdux_ai_insn_read(struct comedi_device *dev,
 	int ret = -EBUSY;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ai_cmd_running)
 		goto ai_read_exit;

@@ -782,7 +782,7 @@ static int usbdux_ai_insn_read(struct comedi_device *dev,
 	}
 
 ai_read_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -795,9 +795,9 @@ static int usbdux_ao_insn_read(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	ret = comedi_readback_insn_read(dev, s, insn, data);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -814,7 +814,7 @@ static int usbdux_ao_insn_write(struct comedi_device *dev,
 	int ret = -EBUSY;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ao_cmd_running)
 		goto ao_write_exit;

@@ -838,7 +838,7 @@ static int usbdux_ao_insn_write(struct comedi_device *dev,
 	}
 
 ao_write_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -854,7 +854,7 @@ static int usbdux_ao_inttrig(struct comedi_device *dev,
 	if (trig_num != cmd->start_arg)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (!devpriv->ao_cmd_running) {
 		devpriv->ao_cmd_running = 1;

@@ -870,7 +870,7 @@ static int usbdux_ao_inttrig(struct comedi_device *dev,
 	}
 
 ao_trig_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 	return ret;
 }
 

@@ -960,7 +960,7 @@ static int usbdux_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	struct comedi_cmd *cmd = &s->async->cmd;
 	int ret = -EBUSY;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ao_cmd_running)
 		goto ao_cmd_exit;

@@ -1002,7 +1002,7 @@ static int usbdux_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	}
 
 ao_cmd_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -1033,7 +1033,7 @@ static int usbdux_dio_insn_bits(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	comedi_dio_update_state(s, data);
 

@@ -1055,7 +1055,7 @@ static int usbdux_dio_insn_bits(struct comedi_device *dev,
 	data[1] = le16_to_cpu(devpriv->insn_buf[1]);
 
 dio_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -1070,7 +1070,7 @@ static int usbdux_counter_read(struct comedi_device *dev,
 	int ret = 0;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	for (i = 0; i < insn->n; i++) {
 		ret = send_dux_commands(dev, USBDUX_CMD_TIMER_RD);

@@ -1084,7 +1084,7 @@ static int usbdux_counter_read(struct comedi_device *dev,
 	}
 
 counter_read_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -1100,7 +1100,7 @@ static int usbdux_counter_write(struct comedi_device *dev,
 	int ret = 0;
 	int i;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	devpriv->dux_commands[1] = chan;
 

@@ -1112,7 +1112,7 @@ static int usbdux_counter_write(struct comedi_device *dev,
 			break;
 	}
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret ? ret : insn->n;
 }

@@ -1148,11 +1148,11 @@ static int usbdux_pwm_cancel(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	/* unlink only if it is really running */
 	usbdux_pwm_stop(dev, devpriv->pwm_cmd_running);
 	ret = send_dux_commands(dev, USBDUX_CMD_PWM_OFF);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -1257,7 +1257,7 @@ static int usbdux_pwm_start(struct comedi_device *dev,
 	struct usbdux_private *devpriv = dev->private;
 	int ret = 0;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->pwm_cmd_running)
 		goto pwm_start_exit;

@@ -1276,7 +1276,7 @@ static int usbdux_pwm_start(struct comedi_device *dev,
 		devpriv->pwm_cmd_running = 0;
 
 pwm_start_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -1576,7 +1576,7 @@ static int usbdux_auto_attach(struct comedi_device *dev,
 	if (!devpriv)
 		return -ENOMEM;
 
-	sema_init(&devpriv->sem, 1);
+	mutex_init(&devpriv->mut);
 
 	usb_set_intfdata(intf, devpriv);
 

@@ -1691,7 +1691,7 @@ static void usbdux_detach(struct comedi_device *dev)
 	if (!devpriv)
 		return;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	/* force unlink all urbs */
 	usbdux_pwm_stop(dev, 1);

@@ -1700,7 +1700,7 @@ static void usbdux_detach(struct comedi_device *dev)
 
 	usbdux_free_usb_buffers(dev);
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 }
 
 static struct comedi_driver usbdux_driver = {
Signature · 8 lines
1.9.1

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cfec30d2021f3462908802a6fe8cbb46cefe213b.1443815181.git.ksenija.stanojevic%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
author
#5 lore raw

PATCH v2 3/4 Staging: comedi: Use mutex instead of semaphore in usbduxfast.c

In reply to Ksenija Stanojevic

Replace binary semaphore with mutex because mutex gives better performance.
This change is safe because the thread that decrements the value of semaphore is also the one that increments it, and acts like a mutex where owner of the lock is the only one that can release the lock.

Signed-off-by Ksenija Stanojevic <[email protected]>

---
Changes in v2:
-change subject line and commit message

 drivers/staging/comedi/drivers/usbduxfast.c | 36 ++++++++++++++---------------
 1 file changed, 18 insertions(+), 18 deletions(-)
drivers/staging/comedi/drivers/usbduxfast.c +18 -18
index c6b2a65..10f94ec 100644
@@ -160,7 +160,7 @@ struct usbduxfast_private {
 	s8 *inbuf;
 	short int ai_cmd_running;	/* asynchronous command is running */
 	int ignore;		/* counter which ignores the first buffers */
-	struct semaphore sem;
+	struct mutex mut;
 };
 
 /*

@@ -221,9 +221,9 @@ static int usbduxfast_ai_cancel(struct comedi_device *dev,
 	struct usbduxfast_private *devpriv = dev->private;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	ret = usbduxfast_ai_stop(dev, 1);
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -444,7 +444,7 @@ static int usbduxfast_ai_inttrig(struct comedi_device *dev,
 	if (trig_num != cmd->start_arg)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (!devpriv->ai_cmd_running) {
 		devpriv->ai_cmd_running = 1;

@@ -452,14 +452,14 @@ static int usbduxfast_ai_inttrig(struct comedi_device *dev,
 		if (ret < 0) {
 			dev_err(dev->class_dev, "urbSubmit: err=%d\n", ret);
 			devpriv->ai_cmd_running = 0;
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 		s->async->inttrig = NULL;
 	} else {
 		dev_err(dev->class_dev, "ai is already running\n");
 	}
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 	return 1;
 }
 

@@ -472,7 +472,7 @@ static int usbduxfast_ai_cmd(struct comedi_device *dev,
 	int j, ret;
 	long steps, steps_tmp;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 	if (devpriv->ai_cmd_running) {
 		ret = -EBUSY;
 		goto cmd_exit;

@@ -751,7 +751,7 @@ static int usbduxfast_ai_cmd(struct comedi_device *dev,
 	}
 
 cmd_exit:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -772,12 +772,12 @@ static int usbduxfast_ai_insn_read(struct comedi_device *dev,
 	int i, j, n, actual_length;
 	int ret;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	if (devpriv->ai_cmd_running) {
 		dev_err(dev->class_dev,
 			"ai_insn_read not possible, async cmd is running\n");
-		up(&devpriv->sem);
+		mutex_unlock(&devpriv->mut);
 		return -EBUSY;
 	}
 

@@ -799,7 +799,7 @@ static int usbduxfast_ai_insn_read(struct comedi_device *dev,
 
 	ret = usbduxfast_send_cmd(dev, SENDADCOMMANDS);
 	if (ret < 0) {
-		up(&devpriv->sem);
+		mutex_unlock(&devpriv->mut);
 		return ret;
 	}
 

@@ -809,7 +809,7 @@ static int usbduxfast_ai_insn_read(struct comedi_device *dev,
 				   &actual_length, 10000);
 		if (ret < 0) {
 			dev_err(dev->class_dev, "insn timeout, no data\n");
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 	}

@@ -820,13 +820,13 @@ static int usbduxfast_ai_insn_read(struct comedi_device *dev,
 				   &actual_length, 10000);
 		if (ret < 0) {
 			dev_err(dev->class_dev, "insn data error: %d\n", ret);
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return ret;
 		}
 		n = actual_length / sizeof(u16);
 		if ((n % 16) != 0) {
 			dev_err(dev->class_dev, "insn data packet corrupted\n");
-			up(&devpriv->sem);
+			mutex_unlock(&devpriv->mut);
 			return -EINVAL;
 		}
 		for (j = chan; (j < n) && (i < insn->n); j = j + 16) {

@@ -835,7 +835,7 @@ static int usbduxfast_ai_insn_read(struct comedi_device *dev,
 		}
 	}
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return insn->n;
 }

@@ -930,7 +930,7 @@ static int usbduxfast_auto_attach(struct comedi_device *dev,
 	if (!devpriv)
 		return -ENOMEM;
 
-	sema_init(&devpriv->sem, 1);
+	mutex_init(&devpriv->mut);
 	usb_set_intfdata(intf, devpriv);
 
 	devpriv->duxbuf = kmalloc(SIZEOFDUXBUF, GFP_KERNEL);

@@ -989,7 +989,7 @@ static void usbduxfast_detach(struct comedi_device *dev)
 	if (!devpriv)
 		return;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	usb_set_intfdata(intf, NULL);
 

@@ -1003,7 +1003,7 @@ static void usbduxfast_detach(struct comedi_device *dev)
 
 	kfree(devpriv->duxbuf);
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 }
 
 static struct comedi_driver usbduxfast_driver = {
Signature · 1 line
1.9.1
author
#6 lore raw

PATCH v2 4/4 Staging: comedi: Use mutex instead of semaphore in ni_usb6501.c

In reply to Ksenija Stanojevic

Replace binary semaphore with mutex because mutex gives better performance.
This change is safe because the thread that decrements the value of semaphore is also the one that increments it, and acts like a mutex where owner of the lock is the only one that can release the lock.

Signed-off-by Ksenija Stanojevic <[email protected]>

---
Changes in v2:
-change subject line and commit message

 drivers/staging/comedi/drivers/ni_usb6501.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/staging/comedi/drivers/ni_usb6501.c +8 -8
index 88de8da..95b537a 100644
@@ -166,7 +166,7 @@ enum commands {
 struct ni6501_private {
 	struct usb_endpoint_descriptor *ep_rx;
 	struct usb_endpoint_descriptor *ep_tx;
-	struct semaphore sem;
+	struct mutex mut;
 	u8 *usb_rx_buf;
 	u8 *usb_tx_buf;
 };

@@ -183,7 +183,7 @@ static int ni6501_port_command(struct comedi_device *dev, int command,
 	if (command != SET_PORT_DIR && !bitmap)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	switch (command) {
 	case READ_PORT:

@@ -248,7 +248,7 @@ static int ni6501_port_command(struct comedi_device *dev, int command,
 		ret = -EINVAL;
 	}
 end:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -265,7 +265,7 @@ static int ni6501_counter_command(struct comedi_device *dev, int command,
 	if ((command == READ_COUNTER || command ==  WRITE_COUNTER) && !val)
 		return -EINVAL;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	switch (command) {
 	case START_COUNTER:

@@ -338,7 +338,7 @@ static int ni6501_counter_command(struct comedi_device *dev, int command,
 		ret = -EINVAL;
 	}
 end:
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 
 	return ret;
 }

@@ -535,7 +535,7 @@ static int ni6501_auto_attach(struct comedi_device *dev,
 	if (ret)
 		return ret;
 
-	sema_init(&devpriv->sem, 1);
+	mutex_init(&devpriv->mut);
 	usb_set_intfdata(intf, devpriv);
 
 	ret = comedi_alloc_subdevices(dev, 2);

@@ -573,14 +573,14 @@ static void ni6501_detach(struct comedi_device *dev)
 	if (!devpriv)
 		return;
 
-	down(&devpriv->sem);
+	mutex_lock(&devpriv->mut);
 
 	usb_set_intfdata(intf, NULL);
 
 	kfree(devpriv->usb_rx_buf);
 	kfree(devpriv->usb_tx_buf);
 
-	up(&devpriv->sem);
+	mutex_unlock(&devpriv->mut);
 }
 
 static struct comedi_driver ni6501_driver = {
Signature · 1 line
1.9.1