[2/2] block: remove retry loop in ioc_release_fn()
diff mbox series

Message ID 20200619151718.22338-3-john.ogness@linutronix.de
State New
Headers show
Series
  • block: remove retry loop
Related show

Commit Message

John Ogness June 19, 2020, 3:17 p.m. UTC
The reverse-order double lock dance in ioc_release_fn() is using a
retry loop. This is a problem on PREEMPT_RT because it could preempt
the task that would release q->queue_lock and thus live lock in the
retry loop.

RCU is already managing the freeing of the request queue and icq. If
the trylock fails, use RCU to guarantee that the request queue and
icq are not freed and re-acquire the locks in the correct order,
allowing forward progress.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 block/blk-ioc.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Daniel Wagner June 23, 2020, 2:30 p.m. UTC | #1
On Fri, Jun 19, 2020 at 05:23:18PM +0206, John Ogness wrote:
> The reverse-order double lock dance in ioc_release_fn() is using a
> retry loop. This is a problem on PREEMPT_RT because it could preempt
> the task that would release q->queue_lock and thus live lock in the
> retry loop.
> 
> RCU is already managing the freeing of the request queue and icq. If
> the trylock fails, use RCU to guarantee that the request queue and
> icq are not freed and re-acquire the locks in the correct order,
> allowing forward progress.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Again, after starring on it for while and reading up and down,
I'd say, it looks good. Also a quick test run with blktests and
lockdep enabled didn't produce any warnings.

Reviewed-by: Daniel Wagner <dwagner@suse.de>

Patch
diff mbox series

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 5dbcfa1b872e..57299f860d41 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -107,9 +107,23 @@  static void ioc_release_fn(struct work_struct *work)
 			ioc_destroy_icq(icq);
 			spin_unlock(&q->queue_lock);
 		} else {
-			spin_unlock_irq(&ioc->lock);
-			cpu_relax();
-			spin_lock_irq(&ioc->lock);
+			/* Make sure q and icq cannot be freed. */
+			rcu_read_lock();
+
+			/* Re-acquire the locks in the correct order. */
+			spin_unlock(&ioc->lock);
+			spin_lock(&q->queue_lock);
+			spin_lock(&ioc->lock);
+
+			/*
+			 * The icq may have been destroyed when the ioc lock
+			 * was released.
+			 */
+			if (!(icq->flags & ICQ_DESTROYED))
+				ioc_destroy_icq(icq);
+
+			spin_unlock(&q->queue_lock);
+			rcu_read_unlock();
 		}
 	}