* ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi")
@ 2020-06-09 10:00 Michael Ellerman
2020-06-09 12:04 ` Christoph Hellwig
2020-06-09 15:42 ` Christoph Hellwig
0 siblings, 2 replies; 7+ messages in thread
From: Michael Ellerman @ 2020-06-09 10:00 UTC (permalink / raw)
To: brking, Christoph Hellwig, Jens Axboe, linux-scsi, LKML,
linuxppc-dev, linux-block, linux-ide
Hi all,
I'm seeing crashes on powerpc with the ipr driver, which I'm fairly sure
are due to dma_need_drain being NULL.
The backtrace is:
scsi_init_io+0x1d8/0x350
scsi_queue_rq+0x7a4/0xc30
blk_mq_dispatch_rq_list+0x1b0/0x910
blk_mq_sched_dispatch_requests+0x154/0x270
__blk_mq_run_hw_queue+0xa0/0x160
__blk_mq_delay_run_hw_queue+0x244/0x250
blk_mq_sched_insert_request+0x13c/0x250
blk_execute_rq_nowait+0x88/0xb0
blk_execute_rq+0x5c/0xf0
__scsi_execute+0x10c/0x270
scsi_mode_sense+0x144/0x440
sr_probe+0x2e8/0x810
really_probe+0x12c/0x580
driver_probe_device+0x88/0x170
device_driver_attach+0x11c/0x130
__driver_attach+0xac/0x190
bus_for_each_dev+0xa8/0x130
driver_attach+0x34/0x50
bus_add_driver+0x170/0x2b0
driver_register+0xb4/0x1c0
scsi_register_driver+0x2c/0x40
init_sr+0x4c/0x80
do_one_initcall+0x60/0x2b0
kernel_init_freeable+0x2e0/0x3a0
kernel_init+0x2c/0x148
ret_from_kernel_thread+0x5c/0x74
And looking at the disassembly I think it's coming from:
static inline bool scsi_cmd_needs_dma_drain(struct scsi_device *sdev,
struct request *rq)
{
return sdev->dma_drain_len && blk_rq_is_passthrough(rq) &&
!op_is_write(req_op(rq)) &&
sdev->host->hostt->dma_need_drain(rq);
^^^^^^^^^^^^^^
}
Bisect agrees:
# first bad commit: [cc97923a5bccc776851c242b61015faf288d5c22] block: move dma drain handling to scsi
And looking at ipr.c, it constructs its scsi_host_template manually,
without using any of the macros that end up calling __ATA_BASE_SHT,
which populates dma_need_drain.
The obvious fix below works, the system boots and seems to be operating
normally, but I don't know enough (anything) about SCSI to say if it's
actually the correct fix.
cheers
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 7d77997d26d4..7d86f4ca266c 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6731,6 +6731,7 @@ static struct scsi_host_template driver_template = {
.compat_ioctl = ipr_ioctl,
#endif
.queuecommand = ipr_queuecommand,
+ .dma_need_drain = ata_scsi_dma_need_drain,
.eh_abort_handler = ipr_eh_abort,
.eh_device_reset_handler = ipr_eh_dev_reset,
.eh_host_reset_handler = ipr_eh_host_reset,
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") 2020-06-09 10:00 ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") Michael Ellerman @ 2020-06-09 12:04 ` Christoph Hellwig 2020-06-09 15:42 ` Christoph Hellwig 1 sibling, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2020-06-09 12:04 UTC (permalink / raw) To: Michael Ellerman Cc: brking, Christoph Hellwig, Jens Axboe, linux-scsi, LKML, linuxppc-dev, linux-block, linux-ide On Tue, Jun 09, 2020 at 08:00:35PM +1000, Michael Ellerman wrote: > Hi all, > > I'm seeing crashes on powerpc with the ipr driver, which I'm fairly sure > are due to dma_need_drain being NULL. Ooops, my changes completely forgot about SAS attached ATAPI devices. I'll cook up a fix in a bit. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") @ 2020-06-09 12:04 ` Christoph Hellwig 0 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2020-06-09 12:04 UTC (permalink / raw) To: Michael Ellerman Cc: Jens Axboe, linux-scsi, LKML, linux-block, linux-ide, brking, linuxppc-dev, Christoph Hellwig On Tue, Jun 09, 2020 at 08:00:35PM +1000, Michael Ellerman wrote: > Hi all, > > I'm seeing crashes on powerpc with the ipr driver, which I'm fairly sure > are due to dma_need_drain being NULL. Ooops, my changes completely forgot about SAS attached ATAPI devices. I'll cook up a fix in a bit. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") 2020-06-09 10:00 ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") Michael Ellerman @ 2020-06-09 15:42 ` Christoph Hellwig 2020-06-09 15:42 ` Christoph Hellwig 1 sibling, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2020-06-09 15:42 UTC (permalink / raw) To: Michael Ellerman Cc: brking, Christoph Hellwig, Jens Axboe, linux-scsi, LKML, linuxppc-dev, linux-block, linux-ide Can you try this patch? --- From 1c9913360a0494375c5655b133899cb4323bceb4 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig <hch@lst.de> Date: Tue, 9 Jun 2020 14:07:31 +0200 Subject: scsi: wire up ata_scsi_dma_need_drain for SAS HBA drivers We need ata_scsi_dma_need_drain for all drivers wired up to drive ATAPI devices through libata. That also includes the SAS HBA drivers in addition to native libata HBA drivers. Fixes: cc97923a5bcc ("block: move dma drain handling to scsi") Reported-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/scsi/aic94xx/aic94xx_init.c | 1 + drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 1 + drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 1 + drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 1 + drivers/scsi/ipr.c | 1 + drivers/scsi/isci/init.c | 1 + drivers/scsi/mvsas/mv_init.c | 1 + drivers/scsi/pm8001/pm8001_init.c | 1 + 8 files changed, 8 insertions(+) diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c index d022407e5645c7..bef47f38dd0dbc 100644 --- a/drivers/scsi/aic94xx/aic94xx_init.c +++ b/drivers/scsi/aic94xx/aic94xx_init.c @@ -40,6 +40,7 @@ static struct scsi_host_template aic94xx_sht = { /* .name is initialized */ .name = "aic94xx", .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = asd_scan_finished, diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c index 2e1718f9ade218..09a7669dad4c67 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c @@ -1756,6 +1756,7 @@ static struct scsi_host_template sht_v1_hw = { .proc_name = DRV_NAME, .module = THIS_MODULE, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = hisi_sas_slave_configure, .scan_finished = hisi_sas_scan_finished, diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c index e7e7849a4c14e2..968d3870235359 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c @@ -3532,6 +3532,7 @@ static struct scsi_host_template sht_v2_hw = { .proc_name = DRV_NAME, .module = THIS_MODULE, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = hisi_sas_slave_configure, .scan_finished = hisi_sas_scan_finished, diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 3e6b78a1f993b9..55e2321a65bc5f 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -3075,6 +3075,7 @@ static struct scsi_host_template sht_v3_hw = { .proc_name = DRV_NAME, .module = THIS_MODULE, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = hisi_sas_slave_configure, .scan_finished = hisi_sas_scan_finished, diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 7d77997d26d457..7d86f4ca266c86 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -6731,6 +6731,7 @@ static struct scsi_host_template driver_template = { .compat_ioctl = ipr_ioctl, #endif .queuecommand = ipr_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .eh_abort_handler = ipr_eh_abort, .eh_device_reset_handler = ipr_eh_dev_reset, .eh_host_reset_handler = ipr_eh_host_reset, diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c index 974c3b9116d5ba..085e285f427d93 100644 --- a/drivers/scsi/isci/init.c +++ b/drivers/scsi/isci/init.c @@ -153,6 +153,7 @@ static struct scsi_host_template isci_sht = { .name = DRV_NAME, .proc_name = DRV_NAME, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = isci_host_scan_finished, diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c index 5973eed9493820..b0de3bdb01db06 100644 --- a/drivers/scsi/mvsas/mv_init.c +++ b/drivers/scsi/mvsas/mv_init.c @@ -33,6 +33,7 @@ static struct scsi_host_template mvs_sht = { .module = THIS_MODULE, .name = DRV_NAME, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = mvs_scan_finished, diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index a8f5344fdfda2a..9e99262a2b9dd3 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -87,6 +87,7 @@ static struct scsi_host_template pm8001_sht = { .module = THIS_MODULE, .name = DRV_NAME, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = pm8001_scan_finished, -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") @ 2020-06-09 15:42 ` Christoph Hellwig 0 siblings, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2020-06-09 15:42 UTC (permalink / raw) To: Michael Ellerman Cc: Jens Axboe, linux-scsi, LKML, linux-block, linux-ide, brking, linuxppc-dev, Christoph Hellwig Can you try this patch? --- From 1c9913360a0494375c5655b133899cb4323bceb4 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig <hch@lst.de> Date: Tue, 9 Jun 2020 14:07:31 +0200 Subject: scsi: wire up ata_scsi_dma_need_drain for SAS HBA drivers We need ata_scsi_dma_need_drain for all drivers wired up to drive ATAPI devices through libata. That also includes the SAS HBA drivers in addition to native libata HBA drivers. Fixes: cc97923a5bcc ("block: move dma drain handling to scsi") Reported-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/scsi/aic94xx/aic94xx_init.c | 1 + drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 1 + drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 1 + drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 1 + drivers/scsi/ipr.c | 1 + drivers/scsi/isci/init.c | 1 + drivers/scsi/mvsas/mv_init.c | 1 + drivers/scsi/pm8001/pm8001_init.c | 1 + 8 files changed, 8 insertions(+) diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c index d022407e5645c7..bef47f38dd0dbc 100644 --- a/drivers/scsi/aic94xx/aic94xx_init.c +++ b/drivers/scsi/aic94xx/aic94xx_init.c @@ -40,6 +40,7 @@ static struct scsi_host_template aic94xx_sht = { /* .name is initialized */ .name = "aic94xx", .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = asd_scan_finished, diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c index 2e1718f9ade218..09a7669dad4c67 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c @@ -1756,6 +1756,7 @@ static struct scsi_host_template sht_v1_hw = { .proc_name = DRV_NAME, .module = THIS_MODULE, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = hisi_sas_slave_configure, .scan_finished = hisi_sas_scan_finished, diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c index e7e7849a4c14e2..968d3870235359 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c @@ -3532,6 +3532,7 @@ static struct scsi_host_template sht_v2_hw = { .proc_name = DRV_NAME, .module = THIS_MODULE, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = hisi_sas_slave_configure, .scan_finished = hisi_sas_scan_finished, diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 3e6b78a1f993b9..55e2321a65bc5f 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -3075,6 +3075,7 @@ static struct scsi_host_template sht_v3_hw = { .proc_name = DRV_NAME, .module = THIS_MODULE, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = hisi_sas_slave_configure, .scan_finished = hisi_sas_scan_finished, diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 7d77997d26d457..7d86f4ca266c86 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -6731,6 +6731,7 @@ static struct scsi_host_template driver_template = { .compat_ioctl = ipr_ioctl, #endif .queuecommand = ipr_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .eh_abort_handler = ipr_eh_abort, .eh_device_reset_handler = ipr_eh_dev_reset, .eh_host_reset_handler = ipr_eh_host_reset, diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c index 974c3b9116d5ba..085e285f427d93 100644 --- a/drivers/scsi/isci/init.c +++ b/drivers/scsi/isci/init.c @@ -153,6 +153,7 @@ static struct scsi_host_template isci_sht = { .name = DRV_NAME, .proc_name = DRV_NAME, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = isci_host_scan_finished, diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c index 5973eed9493820..b0de3bdb01db06 100644 --- a/drivers/scsi/mvsas/mv_init.c +++ b/drivers/scsi/mvsas/mv_init.c @@ -33,6 +33,7 @@ static struct scsi_host_template mvs_sht = { .module = THIS_MODULE, .name = DRV_NAME, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = mvs_scan_finished, diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index a8f5344fdfda2a..9e99262a2b9dd3 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -87,6 +87,7 @@ static struct scsi_host_template pm8001_sht = { .module = THIS_MODULE, .name = DRV_NAME, .queuecommand = sas_queuecommand, + .dma_need_drain = ata_scsi_dma_need_drain, .target_alloc = sas_target_alloc, .slave_configure = sas_slave_configure, .scan_finished = pm8001_scan_finished, -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") 2020-06-09 15:42 ` Christoph Hellwig @ 2020-06-10 5:43 ` Michael Ellerman -1 siblings, 0 replies; 7+ messages in thread From: Michael Ellerman @ 2020-06-10 5:43 UTC (permalink / raw) To: Christoph Hellwig Cc: brking, Christoph Hellwig, Jens Axboe, linux-scsi, LKML, linuxppc-dev, linux-block, linux-ide Christoph Hellwig <hch@lst.de> writes: > Can you try this patch? > > --- > From 1c9913360a0494375c5655b133899cb4323bceb4 Mon Sep 17 00:00:00 2001 > From: Christoph Hellwig <hch@lst.de> > Date: Tue, 9 Jun 2020 14:07:31 +0200 > Subject: scsi: wire up ata_scsi_dma_need_drain for SAS HBA drivers > > We need ata_scsi_dma_need_drain for all drivers wired up to drive ATAPI > devices through libata. That also includes the SAS HBA drivers in > addition to native libata HBA drivers. > > Fixes: cc97923a5bcc ("block: move dma drain handling to scsi") > Reported-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Christoph Hellwig <hch@lst.de> Yep that works for me here with ipr. Tested-by: Michael Ellerman <mpe@ellerman.id.au> cheers ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") @ 2020-06-10 5:43 ` Michael Ellerman 0 siblings, 0 replies; 7+ messages in thread From: Michael Ellerman @ 2020-06-10 5:43 UTC (permalink / raw) To: Christoph Hellwig Cc: Jens Axboe, linux-scsi, LKML, linux-block, linux-ide, brking, linuxppc-dev, Christoph Hellwig Christoph Hellwig <hch@lst.de> writes: > Can you try this patch? > > --- > From 1c9913360a0494375c5655b133899cb4323bceb4 Mon Sep 17 00:00:00 2001 > From: Christoph Hellwig <hch@lst.de> > Date: Tue, 9 Jun 2020 14:07:31 +0200 > Subject: scsi: wire up ata_scsi_dma_need_drain for SAS HBA drivers > > We need ata_scsi_dma_need_drain for all drivers wired up to drive ATAPI > devices through libata. That also includes the SAS HBA drivers in > addition to native libata HBA drivers. > > Fixes: cc97923a5bcc ("block: move dma drain handling to scsi") > Reported-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Christoph Hellwig <hch@lst.de> Yep that works for me here with ipr. Tested-by: Michael Ellerman <mpe@ellerman.id.au> cheers ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-10 5:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-09 10:00 ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi") Michael Ellerman
2020-06-09 12:04 ` Christoph Hellwig
2020-06-09 12:04 ` Christoph Hellwig
2020-06-09 15:42 ` Christoph Hellwig
2020-06-09 15:42 ` Christoph Hellwig
2020-06-10 5:43 ` Michael Ellerman
2020-06-10 5:43 ` Michael Ellerman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.