drivers/edac: fix workq reset deadlock

Fix mutex locking deadlock on the device controller linked list.  Was calling
a lock then a function that could call the same lock.  Moved the cancel workq
function to outside the lock

Added some short circuit logic in the workq code

Added comments of description

Code tidying

Signed-off-by: Doug Thompson <dougthompson@xmission.com>
Cc: Greg KH <greg@kroah.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 47 insertions, 9 deletions

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 173f4ba..7e37237 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -32,7 +32,9 @@
 #include "edac_core.h"
 #include "edac_module.h"
 
-/* lock to memory controller's control array 'edac_device_list' */
+/* lock for the list: 'edac_device_list', manipulation of this list
+ * is protected by the 'device_ctls_mutex' lock
+ */
 static DEFINE_MUTEX(device_ctls_mutex);
 static struct list_head edac_device_list = LIST_HEAD_INIT(edac_device_list);
 
@@ -386,6 +388,14 @@ EXPORT_SYMBOL_GPL(edac_device_find);
 /*
  * edac_device_workq_function
  *	performs the operation scheduled by a workq request
+ *
+ *	this workq is embedded within an edac_device_ctl_info
+ *	structure, that needs to be polled for possible error events.
+ *
+ *	This operation is to acquire the list mutex lock
+ *	(thus preventing insertation or deletion)
+ *	and then call the device's poll function IFF this device is
+ *	running polled and there is a poll function defined.
  */
 static void edac_device_workq_function(struct work_struct *work_req)
 {
@@ -403,8 +413,17 @@ static void edac_device_workq_function(struct work_struct *work_req)
 
 	mutex_unlock(&device_ctls_mutex);
 
-	/* Reschedule */
-	queue_delayed_work(edac_workqueue, &edac_dev->work, edac_dev->delay);
+	/* Reschedule the workq for the next time period to start again
+	 * if the number of msec is for 1 sec, then adjust to the next
+	 * whole one second to save timers fireing all over the period
+	 * between integral seconds
+	 */
+	if (edac_dev->poll_msec == 1000)
+		queue_delayed_work(edac_workqueue, &edac_dev->work,
+				round_jiffies(edac_dev->delay));
+	else
+		queue_delayed_work(edac_workqueue, &edac_dev->work,
+				edac_dev->delay);
 }
 
 /*
@@ -417,11 +436,26 @@ void edac_device_workq_setup(struct edac_device_ctl_info *edac_dev,
 {
 	debugf0("%s()\n", __func__);
 
+	/* take the arg 'msec' and set it into the control structure
+	 * to used in the time period calculation
+	 * then calc the number of jiffies that represents
+	 */
 	edac_dev->poll_msec = msec;
-	edac_dev->delay = msecs_to_jiffies(msec);	/* Calc delay jiffies */
+	edac_dev->delay = msecs_to_jiffies(msec);
 
 	INIT_DELAYED_WORK(&edac_dev->work, edac_device_workq_function);
-	queue_delayed_work(edac_workqueue, &edac_dev->work, edac_dev->delay);
+
+	/* optimize here for the 1 second case, which will be normal value, to
+	 * fire ON the 1 second time event. This helps reduce all sorts of
+	 * timers firing on sub-second basis, while they are happy
+	 * to fire together on the 1 second exactly
+	 */
+	if (edac_dev->poll_msec == 1000)
+		queue_delayed_work(edac_workqueue, &edac_dev->work,
+				round_jiffies(edac_dev->delay));
+	else
+		queue_delayed_work(edac_workqueue, &edac_dev->work,
+				edac_dev->delay);
 }
 
 /*
@@ -441,16 +475,20 @@ void edac_device_workq_teardown(struct edac_device_ctl_info *edac_dev)
 
 /*
  * edac_device_reset_delay_period
+ *
+ *	need to stop any outstanding workq queued up at this time
+ *	because we will be resetting the sleep time.
+ *	Then restart the workq on the new delay
  */
-
 void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev,
 					unsigned long value)
 {
-	mutex_lock(&device_ctls_mutex);
-
-	/* cancel the current workq request */
+	/* cancel the current workq request, without the mutex lock */
 	edac_device_workq_teardown(edac_dev);
 
+	/* acquire the mutex before doing the workq setup */
+	mutex_lock(&device_ctls_mutex);
+
 	/* restart the workq request, with new delay value */
 	edac_device_workq_setup(edac_dev, value);