[dm-devel] dm-mq and end_clone_request()

[Johannes Thumshirn]

On Mon, Aug 08, 2016 at 08:45:59AM -0400, Laurence Oberman wrote:
> 
> 
> ----- Original Message -----
> > From: "Bart Van Assche" <bvanassche at acm.org>
> > To: "Laurence Oberman" <loberman at redhat.com>
> > Cc: "Mike Snitzer" <snitzer at redhat.com>, dm-devel at redhat.com, linux-scsi at vger.kernel.org
> > Sent: Sunday, August 7, 2016 6:31:11 PM
> > Subject: Re: [dm-devel] dm-mq and end_clone_request()
> > 
> > On 08/06/16 07:47, Laurence Oberman wrote:
> > > [66813.933246] Workqueue: srp_remove srp_remove_work [ib_srp]
> > > [ ... ]
> > > [66815.152051]  [<ffffffff814ac790>] scsi_forget_host+0x60/0x70
> > > [66815.183939]  [<ffffffff814a0137>] scsi_remove_host+0x77/0x110
> > > [66815.216152]  [<ffffffffa0677be0>] srp_remove_work+0x90/0x200 [ib_srp]
> > > [66815.253221]  [<ffffffff810a2e72>] process_one_work+0x152/0x400
> > > [66815.286221]  [<ffffffff810a3765>] worker_thread+0x125/0x4b0
> > > [66815.317313]  [<ffffffff810a3640>] ? rescuer_thread+0x380/0x380
> > > [66815.349770]  [<ffffffff810a9298>] kthread+0xd8/0xf0
> > > [66815.376082]  [<ffffffff816c6b3f>] ret_from_fork+0x1f/0x40
> > > [66815.404767]  [<ffffffff810a91c0>] ? kthread_park+0x60/0x60
> > 
> > Hello Laurence,
> > 
> > This is a callstack I have not yet encountered myself during any test.
> > Please provide the output of the following commands:
> > $ gdb /lib/modules/$(uname -r)/build/vmlinux
> > (gdb) list *(scsi_forget_host+0x60)
> > 
> > Thanks,
> > 
> > Bart.
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> [loberman at jumptest1 linux]$ gdb vmlinux
> GNU gdb (GDB) Red Hat Enterprise Linux 7.6.1-80.el7
> Copyright (C) 2013 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "x86_64-redhat-linux-gnu".
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>...
> Reading symbols from /home/loberman/bart/linux/vmlinux...done.
> (gdb) list *(scsi_forget_host+0x60)
> 0xffffffff814ac790 is in scsi_forget_host (drivers/scsi/scsi_scan.c:1895).
> 1890		list_for_each_entry(sdev, &shost->__devices, siblings) {
> 1891			if (sdev->sdev_state == SDEV_DEL)
> 1892				continue;
> 1893			spin_unlock_irqrestore(shost->host_lock, flags);
> 1894			__scsi_remove_device(sdev);
> 1895			goto restart;
> 1896		}
> 1897		spin_unlock_irqrestore(shost->host_lock, flags);
> 1898	}
> 1899	


So __scsi_remove_device() is also checking for sdev->sdev_state == SDEV_DEL
and returns if so. If it would have the chance to do so the goto restart would
be hit and we'd reatart the list traverse. The if in turn just continues and
I've seen endless loops with this pattern (check the 40998193560 ->
90a88d6ef88edc -> f05795d3d771f30a7bd commit chain).

Might want to give the below patch a shot?


>From fee838ebfea88b581994b3f855eab8da20b07fc9 Mon Sep 17 00:00:00 2001
From: Johannes Thumshirn <jthumshirn at suse.de>
Date: Mon, 8 Aug 2016 15:41:09 +0200
Subject: [PATCH] scsi: restart full list search when re-encountering a deleted device

__scsi_remove_device() already checks for reentrency with a deleted device,
so there's no need to do it in scsi_forget_host() as well.

Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
---
 drivers/scsi/scsi_scan.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e0a78f5..1c5a4d6 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1890,8 +1890,6 @@ void scsi_forget_host(struct Scsi_Host *shost)
  restart:
 	spin_lock_irqsave(shost->host_lock, flags);
 	list_for_each_entry(sdev, &shost->__devices, siblings) {
-		if (sdev->sdev_state == SDEV_DEL)
-			continue;
 		spin_unlock_irqrestore(shost->host_lock, flags);
 		__scsi_remove_device(sdev);
 		goto restart;
-- 
2.9.2


-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850