[PATCH] gpib: Move stuck SRQ update under lock

Gui-Dong Han posted 1 patch 2 days, 11 hours ago
drivers/gpib/common/gpib_os.c | 21 +++++++++++----------
drivers/gpib/common/iblib.c   |  3 ---
2 files changed, 11 insertions(+), 13 deletions(-)
[PATCH] gpib: Move stuck SRQ update under lock
Posted by Gui-Dong Han 2 days, 11 hours ago
Move the stuck SRQ state update into autopoll_all_devices() and keep it
under big_gpib_mutex. Except for initialization, keep the stuck_srq users
under this mutex.

autopoll_all_devices() is only called by autospoll_thread(), so there is
no need to return to autospoll_thread() and set this state after dropping
big_gpib_mutex.

Without the mutex, a newly opened device can clear stuck_srq and have
that clear overwritten by the previous autospoll result:

  autospoll: serial_poll_all() returns 0 and unlocks big_gpib_mutex
  open_dev_ioctl: open new device and clear stuck_srq
                  with big_gpib_mutex held
  autospoll: set stuck_srq

That leaves the board marked stuck again after the new device is opened.
autospoll_wait_should_wake_up() then refuses to poll while stuck_srq is
set, so later SRQ handling can be mistakenly suppressed.

Without the mutex, atomic_set() and set_bit() only make individual
updates atomic. They do not order the two updates or make stuck_srq and
status visible as a consistent pair. Taking big_gpib_mutex serializes the
state transition with the other runtime users.

Keep the existing wakeup behavior unchanged and only move the stuck SRQ
state update under the mutex.

Fixes: 9dde4559e939 ("staging: gpib: Add GPIB common core driver")
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
Found by auditing atomic operations used for synchronization.
A similar fix can be found in 6df8e84aa6b5.
---
 drivers/gpib/common/gpib_os.c | 21 +++++++++++----------
 drivers/gpib/common/iblib.c   |  3 ---
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpib/common/gpib_os.c b/drivers/gpib/common/gpib_os.c
index 5909274ddc12..82844eb25917 100644
--- a/drivers/gpib/common/gpib_os.c
+++ b/drivers/gpib/common/gpib_os.c
@@ -289,18 +289,19 @@ int autopoll_all_devices(struct gpib_board *board)
 	dev_dbg(board->gpib_dev, "autopoll has board lock\n");
 
 	retval = serial_poll_all(board, serial_timeout);
-	if (retval < 0)	{
-		mutex_unlock(&board->big_gpib_mutex);
-		mutex_unlock(&board->user_mutex);
-		return retval;
+	if (retval >= 0) {
+		dev_dbg(board->gpib_dev, "complete\n");
+		/*
+		 * need to wake wait queue in case someone is
+		 * waiting on RQS
+		 */
+		wake_up_interruptible(&board->wait);
 	}
 
-	dev_dbg(board->gpib_dev, "complete\n");
-	/*
-	 * need to wake wait queue in case someone is
-	 * waiting on RQS
-	 */
-	wake_up_interruptible(&board->wait);
+	if (retval <= 0) {
+		atomic_set(&board->stuck_srq, 1);
+		set_bit(SRQI_NUM, &board->status);
+	}
 	mutex_unlock(&board->big_gpib_mutex);
 	mutex_unlock(&board->user_mutex);
 
diff --git a/drivers/gpib/common/iblib.c b/drivers/gpib/common/iblib.c
index b672dd6aad25..511e1d61c1fb 100644
--- a/drivers/gpib/common/iblib.c
+++ b/drivers/gpib/common/iblib.c
@@ -193,9 +193,6 @@ static int autospoll_thread(void *board_void)
 		}
 		if (retval <= 0) {
 			dev_err(board->gpib_dev, "stuck SRQ\n");
-
-			atomic_set(&board->stuck_srq, 1);	// XXX could be better
-			set_bit(SRQI_NUM, &board->status);
 		}
 	}
 	return retval;
-- 
2.34.1