Re: [PATCH 22/98] mtip32xx: abort commands in flight on surprise removal

[Bart Van Assche <bvanassche@acm.org>]

On 10/28/26 09:41, Jens Axboe wrote:
> +	/*
> +	 * Nobody will complete the commands a removed device has been given,
> +	 * and del_gendisk() waits for them.
> +	 */
> +	if (dd->sr) {
> +		mtip_stop_service_thread(dd);
> +		blk_mq_quiesce_queue(dd->queue);
> +		blk_mq_tagset_busy_iter(&dd->tags, mtip_abort_cmd, dd);
> +		blk_mq_unquiesce_queue(dd->queue);
> +	}
> +
>  	if (test_bit(MTIP_DDF_INIT_DONE_BIT, &dd->dd_flag))
>  		del_gendisk(dd->disk);

Two issues with this ordering:

1. Calling mtip_stop_service_thread(dd) (kthread_stop()) before aborting
   in-flight commands deadlocks with patch 20/98 if
   mtip_service_thread() is blocked inside blk_mark_disk_dead() ->
   fs_bdev_mark_dead() waiting for sb->s_umount while the s_umount
   holder is waiting on those uncompleted NCQ commands.
2. If PCIe link loss happens after mtip_pci_remove() has already checked
   dd->sr (false) and entered del_gendisk(dd->disk),
   MTIP_DDF_REMOVE_PENDING_BIT is already set. mtip_service_thread()
   exits immediately upon seeing MTIP_DDF_REMOVE_PENDING_BIT without
   handling MTIP_PF_SR_BIT or MTIP_PF_TO_ACTIVE_BIT, leaving in-flight
   commands uncompleted forever in del_gendisk().

With the replacement proposed for patch 20/98 (where
mtip_handle_surprise_removal() aborts in-flight commands before calling
blk_mark_disk_dead()), we can instead keep mtip_service_thread() running
until kthread_stop() after del_gendisk(dd->disk) by removing its early
exit on MTIP_DDF_REMOVE_PENDING_BIT (and clearing MTIP_PF_REBUILD_BIT if
remove is pending or mtip_ftl_rebuild_poll(dd) <= 0, since rebuild poll
returns -EFAULT once MTIP_DDF_REMOVE_PENDING_BIT is set). That handles
surprise removal both before and during del_gendisk() from the service
thread without racing two tagset iterations.

Below is the replacement for 22/98 on top of the 20/98 replacement +
unmodified 21/98 (checked with checkpatch.pl and syntax-checked with
Clang; not runtime-tested on mtip32xx hardware):

---
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 757f9b74871e..b9dfa7112c95 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -2504,10 +2504,6 @@ static int mtip_service_thread(void *data)
 			test_bit(MTIP_PF_SVC_THD_STOP_BIT, &port->flags))
 			goto st_out;
 
-		if (unlikely(test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
-				&dd->dd_flag)))
-			goto st_out;
-
 		set_bit(MTIP_PF_SVC_THD_ACTIVE_BIT, &port->flags);
 
 		if (test_and_clear_bit(MTIP_PF_SR_BIT, &port->flags)) {
@@ -2588,7 +2584,9 @@ restart_eh:
 		}
 
 		if (test_bit(MTIP_PF_REBUILD_BIT, &port->flags)) {
-			if (mtip_ftl_rebuild_poll(dd) == 0)
+			if (test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
+				     &dd->dd_flag) ||
+			    mtip_ftl_rebuild_poll(dd) <= 0)
 				clear_bit(MTIP_PF_REBUILD_BIT, &port->flags);
 		}
 	}
@@ -3926,6 +3924,9 @@ static void mtip_pci_remove(struct pci_dev *pdev)
 	set_bit(MTIP_DDF_REMOVE_PENDING_BIT, &dd->dd_flag);
 	mutex_lock(&dd->ioctl_mutex);
 
+	if (dd->sr && !dd->mtip_svc_handler)
+		mtip_handle_surprise_removal(dd);
+
 	if (test_bit(MTIP_DDF_INIT_DONE_BIT, &dd->dd_flag))
 		del_gendisk(dd->disk);