Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1406604

[PATCH 07/24] staging: unisys: visorinput: remove unnecessary locking

Show all headers | View raw

From: Tim Sell <Timothy.Sell@unisys.com>

Locking in the _interrupt() function is NOT necessary so long as we ensure
that interrupts have been stopped whenever we need to pause or resume the
device, which we now do.

While a device is paused, we ensure that interrupts stay disabled, i.e.
that the _interrupt() function will NOT be called, yet remember the desired
state in devdata->interrupts_enabled if open() or close() are called are
called while the device is paused.  Then when the device is resumed, we
restore the actual state of interrupts (i.e., whether _interrupt() is going
to be called or not) to the desired state in devdata->interrupts_enabled.

Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorinput/visorinput.c | 57 +++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index d67cd763..f633985 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -66,6 +66,7 @@ struct visorinput_devdata {
 	struct rw_semaphore lock_visor_dev; /* lock for dev */
 	struct input_dev *visorinput_dev;
 	bool paused;
+	bool interrupts_enabled;
 	unsigned int keycode_table_bytes; /* size of following array */
 	/* for keyboard devices: visorkbd_keycode[] + visorkbd_ext_keycode[] */
 	unsigned char keycode_table[0];
@@ -228,7 +229,21 @@ static int visorinput_open(struct input_dev *visorinput_dev)
 		return -EINVAL;
 	}
 	dev_dbg(&visorinput_dev->dev, "%s opened\n", __func__);
+
+	/*
+	 * If we're not paused, really enable interrupts.
+	 * Regardless of whether we are paused, set a flag indicating
+	 * interrupts should be enabled so when we resume, interrupts
+	 * will really be enabled.
+	 */
+	down_write(&devdata->lock_visor_dev);
+	devdata->interrupts_enabled = true;
+	if (devdata->paused)
+		goto out_unlock;
 	visorbus_enable_channel_interrupts(devdata->dev);
+
+out_unlock:
+	up_write(&devdata->lock_visor_dev);
 	return 0;
 }
 
@@ -243,7 +258,22 @@ static void visorinput_close(struct input_dev *visorinput_dev)
 		return;
 	}
 	dev_dbg(&visorinput_dev->dev, "%s closed\n", __func__);
+
+	/*
+	 * If we're not paused, really disable interrupts.
+	 * Regardless of whether we are paused, set a flag indicating
+	 * interrupts should be disabled so when we resume we will
+	 * not re-enable them.
+	 */
+
+	down_write(&devdata->lock_visor_dev);
+	devdata->interrupts_enabled = false;
+	if (devdata->paused)
+		goto out_unlock;
 	visorbus_disable_channel_interrupts(devdata->dev);
+
+out_unlock:
+	up_write(&devdata->lock_visor_dev);
 }
 
 /*
@@ -438,10 +468,8 @@ visorinput_remove(struct visor_device *dev)
 	 * in visorinput_channel_interrupt()
 	 */
 
-	down_write(&devdata->lock_visor_dev);
 	dev_set_drvdata(&dev->device, NULL);
 	unregister_client_input(devdata->visorinput_dev);
-	up_write(&devdata->lock_visor_dev);
 	kfree(devdata);
 }
 
@@ -529,13 +557,7 @@ visorinput_channel_interrupt(struct visor_device *dev)
 	if (!devdata)
 		return;
 
-	down_write(&devdata->lock_visor_dev);
-	if (devdata->paused) /* don't touch device/channel when paused */
-		goto out_locked;
-
 	visorinput_dev = devdata->visorinput_dev;
-	if (!visorinput_dev)
-		goto out_locked;
 
 	while (visorchannel_signalremove(dev->visorchannel, 0, &r)) {
 		scancode = r.activity.arg1;
@@ -611,8 +633,6 @@ visorinput_channel_interrupt(struct visor_device *dev)
 			break;
 		}
 	}
-out_locked:
-	up_write(&devdata->lock_visor_dev);
 }
 
 static int
@@ -632,6 +652,14 @@ visorinput_pause(struct visor_device *dev,
 		rc = -EBUSY;
 		goto out_locked;
 	}
+	if (devdata->interrupts_enabled)
+		visorbus_disable_channel_interrupts(dev);
+
+	/*
+	 * due to above, at this time no thread of execution will be
+	 * in visorinput_channel_interrupt()
+	 */
+
 	devdata->paused = true;
 	complete_func(dev, 0);
 	rc = 0;
@@ -659,6 +687,15 @@ visorinput_resume(struct visor_device *dev,
 	}
 	devdata->paused = false;
 	complete_func(dev, 0);
+
+	/*
+	 * Re-establish calls to visorinput_channel_interrupt() if that is
+	 * the desired state that we've kept track of in interrupts_enabled
+	 * while the device was paused.
+	 */
+	if (devdata->interrupts_enabled)
+		visorbus_enable_channel_interrupts(dev);
+
 	rc = 0;
 out_locked:
 	up_write(&devdata->lock_visor_dev);
-- 
1.9.1

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 00/24]  Fixes comments made by tglx, then move visorbus to drivers/virt David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 11/24] staging: unisys: visorbus: Remove unnecessary EXPORT_SYMBOL statements David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 18/24] staging: unisys: visorbus: make visorchipset function descriptions more kerneldoc-like David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 24/24] drivers: Add visorbus to the drivers directory David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 14/24] staging: unisys: visorbus: vbusdeviceinfo function descriptions more kerneldoc-like David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 22/24] include: linux: visorbus: Add visorbus to include/linux directory David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 01/24] staging: unisys: visorbus: remove unused module parameters David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 10/24] staging: unisys: visorbus: Make visordriver_callback_lock a mutex David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 09/24] staging: unisys: visorbus: remove periodic_work.h/.c David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 04/24] staging: unisys: visornic: Correct comment spelling mistake David Kershner <david.kershner@unisys.com> - 2016-05-25 05:50 +0200
  [PATCH 08/24] staging: unisys: visorbus: use kernel timer instead of workqueue David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 17/24] staging: unisys: visorbus: make visorchannel function descriptions more kerneldoc-like David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 19/24] staging: unisys: visorbus: Move visorbus-unique functions to private header David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 16/24] staging: unisys: visorbus: make visorbus_private.h function descriptions more kerneldoc-like David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 06/24] staging: unisys: visorbus: removed unused periodic_test_workqueue David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 20/24] staging: unisys: visorbus: Add kerneldoc-style comments for visorbus API David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 07/24] staging: unisys: visorinput: remove unnecessary locking David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 13/24] staging: unisys: Remove reference to unused STANDALONE_CLIENT David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 15/24] staging: unisys: visorbus: make function descriptions more kerneldoc-like David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 05/24] staging: unisys: include: Remove thread-related enum members David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 23/24] Documentation: Move visorbus documentation from staging to Documentation/ David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 02/24] staging: unisys: visorbus: remove unused struct David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 12/24] staging: unisys: visorbus: Remove unused functions David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200
  [PATCH 03/24] staging: unisys: visorbus: modify format string to match argument David Kershner <david.kershner@unisys.com> - 2016-05-25 06:00 +0200

csiph-web