All of lore.kernel.org
 help / color / mirror / Atom feed
* properly communicate queue limits to the DMA layer v2
@ 2019-06-17 12:19 Christoph Hellwig
  2019-06-17 12:19 ` [PATCH 1/8] scsi: add a host / host template field for the virt boundary Christoph Hellwig
                   ` (8 more replies)
  0 siblings, 9 replies; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

Hi Martin,

we've always had a bit of a problem communicating the block layer
queue limits to the DMA layer, which needs to respect them when
an IOMMU that could merge segments is used.  Unfortunately most
drivers don't get this right.  Oddly enough we've been mostly
getting away with it, although lately dma-debug has been catching
a few of those issues.

The segment merging fix for devices with PRP-like structures seems
to have escalated this a bit.  The first patch fixes the actual
report from Sebastian, while the rest fix every drivers that appears
to have the problem based on a code audit looking for drivers using
blk_queue_max_segment_size, blk_queue_segment_boundary or
blk_queue_virt_boundary and calling dma_map_sg eventually.

For SCSI drivers I've taken the blk_queue_virt_boundary setting
to the SCSI core, similar to how we did it for the other two settings
a while ago.  This also deals with the fact that the DMA layer
settings are on a per-device granularity, so the per-device settings
in a few SCSI drivers can't actually work in an IOMMU environment.

It would be nice to eventually pass these limits as arguments to
dma_map_sg, but that is a far too big series for the 5.2 merge
window.

Changes since v1:
 - dropped block layer parts merged by Jens
 - dropped the usb-storage / uas changes, as the virt_boundary usage
   there will be dropped soon
 - reworked the mpt3sas / megaraid_sas changes to keep per-device
   settings

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/8] scsi: add a host / host template field for the virt boundary
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
@ 2019-06-17 12:19 ` Christoph Hellwig
  2019-06-17 20:51   ` Bart Van Assche
  2019-06-18  0:35   ` Ming Lei
  2019-06-17 12:19 ` [PATCH 2/8] scsi: take the DMA max mapping size into account Christoph Hellwig
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

This allows drivers setting it up easily instead of branching out to
block layer calls in slave_alloc, and ensures the upgraded
max_segment_size setting gets picked up by the DMA layer.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/hosts.c     | 3 +++
 drivers/scsi/scsi_lib.c  | 3 ++-
 include/scsi/scsi_host.h | 3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index ff0d8c6a8d0c..55522b7162d3 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -462,6 +462,9 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	else
 		shost->dma_boundary = 0xffffffff;
 
+	if (sht->virt_boundary_mask)
+		shost->virt_boundary_mask = sht->virt_boundary_mask;
+
 	device_initialize(&shost->shost_gendev);
 	dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
 	shost->shost_gendev.bus = &scsi_bus_type;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 65d0a10c76ad..d333bb6b1c59 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1775,7 +1775,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
 	dma_set_seg_boundary(dev, shost->dma_boundary);
 
 	blk_queue_max_segment_size(q, shost->max_segment_size);
-	dma_set_max_seg_size(dev, shost->max_segment_size);
+	blk_queue_virt_boundary(q, shost->virt_boundary_mask);
+	dma_set_max_seg_size(dev, queue_max_segment_size(q));
 
 	/*
 	 * Set a reasonable default alignment:  The larger of 32-byte (dword),
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index a5fcdad4a03e..cc139dbd71e5 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -369,6 +369,8 @@ struct scsi_host_template {
 	 */
 	unsigned long dma_boundary;
 
+	unsigned long virt_boundary_mask;
+
 	/*
 	 * This specifies "machine infinity" for host templates which don't
 	 * limit the transfer size.  Note this limit represents an absolute
@@ -587,6 +589,7 @@ struct Scsi_Host {
 	unsigned int max_sectors;
 	unsigned int max_segment_size;
 	unsigned long dma_boundary;
+	unsigned long virt_boundary_mask;
 	/*
 	 * In scsi-mq mode, the number of hardware queues supported by the LLD.
 	 *
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/8] scsi: take the DMA max mapping size into account
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
  2019-06-17 12:19 ` [PATCH 1/8] scsi: add a host / host template field for the virt boundary Christoph Hellwig
@ 2019-06-17 12:19 ` Christoph Hellwig
  2019-06-17 20:56   ` Bart Van Assche
  2019-06-17 12:19 ` [PATCH 3/8] ufshcd: set max_segment_size in the scsi host template Christoph Hellwig
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

We need to limit the devices max_sectors to what the DMA mapping
implementation can support.  If not we risk running out of swiotlb
buffers easily.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d333bb6b1c59..f233bfd84cd7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1768,6 +1768,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
 		blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
 	}
 
+	shost->max_sectors = min_t(unsigned int, shost->max_sectors,
+			dma_max_mapping_size(dev) << SECTOR_SHIFT);
 	blk_queue_max_hw_sectors(q, shost->max_sectors);
 	if (shost->unchecked_isa_dma)
 		blk_queue_bounce_limit(q, BLK_BOUNCE_ISA);
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/8] ufshcd: set max_segment_size in the scsi host template
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
  2019-06-17 12:19 ` [PATCH 1/8] scsi: add a host / host template field for the virt boundary Christoph Hellwig
  2019-06-17 12:19 ` [PATCH 2/8] scsi: take the DMA max mapping size into account Christoph Hellwig
@ 2019-06-17 12:19 ` Christoph Hellwig
  2019-06-17 20:58   ` Bart Van Assche
  2019-06-17 12:19 ` [PATCH 4/8] storvsc: set virt_boundary_mask " Christoph Hellwig
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

We need to also mirror the value to the device to ensure IOMMU merging
doesn't undo it, and the SCSI host level parameter will ensure that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/ufs/ufshcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 3fe3029617a8..505d625ed28d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4587,8 +4587,6 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
 	struct request_queue *q = sdev->request_queue;
 
 	blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
-	blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
-
 	return 0;
 }
 
@@ -6991,6 +6989,7 @@ static struct scsi_host_template ufshcd_driver_template = {
 	.sg_tablesize		= SG_ALL,
 	.cmd_per_lun		= UFSHCD_CMD_PER_LUN,
 	.can_queue		= UFSHCD_CAN_QUEUE,
+	.max_segment_size	= PRDT_DATA_BYTE_COUNT_MAX,
 	.max_host_blocked	= 1,
 	.track_queue_depth	= 1,
 	.sdev_groups		= ufshcd_driver_groups,
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 4/8] storvsc: set virt_boundary_mask in the scsi host template
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2019-06-17 12:19 ` [PATCH 3/8] ufshcd: set max_segment_size in the scsi host template Christoph Hellwig
@ 2019-06-17 12:19 ` Christoph Hellwig
  2019-06-17 20:59   ` Bart Van Assche
  2019-06-17 12:19 ` [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host Christoph Hellwig
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

This ensures all proper DMA layer handling is taken care of by the
SCSI midlayer.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/storvsc_drv.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index b89269120a2d..7ed6f2fc1446 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1422,9 +1422,6 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 {
 	blk_queue_rq_timeout(sdevice->request_queue, (storvsc_timeout * HZ));
 
-	/* Ensure there are no gaps in presented sgls */
-	blk_queue_virt_boundary(sdevice->request_queue, PAGE_SIZE - 1);
-
 	sdevice->no_write_same = 1;
 
 	/*
@@ -1697,6 +1694,8 @@ static struct scsi_host_template scsi_driver = {
 	.this_id =		-1,
 	/* Make sure we dont get a sg segment crosses a page boundary */
 	.dma_boundary =		PAGE_SIZE-1,
+	/* Ensure there are no gaps in presented sgls */
+	.virt_boundary_mask =	PAGE_SIZE-1,
 	.no_write_same =	1,
 	.track_queue_depth =	1,
 };
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2019-06-17 12:19 ` [PATCH 4/8] storvsc: set virt_boundary_mask " Christoph Hellwig
@ 2019-06-17 12:19 ` Christoph Hellwig
  2019-06-17 17:34   ` Sagi Grimberg
  2019-06-24 22:32     ` Max Gurtovoy
  2019-06-17 12:19 ` [PATCH 6/8] IB/srp: " Christoph Hellwig
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

This ensures all proper DMA layer handling is taken care of by the
SCSI midlayer.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 35 +++++-------------------
 1 file changed, 7 insertions(+), 28 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 9c185a8dabd3..841b66397a57 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -613,6 +613,7 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
 	struct Scsi_Host *shost;
 	struct iser_conn *iser_conn = NULL;
 	struct ib_conn *ib_conn;
+	struct ib_device *ib_dev;
 	u32 max_fr_sectors;
 
 	shost = iscsi_host_alloc(&iscsi_iser_sht, 0, 0);
@@ -643,16 +644,19 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
 		}
 
 		ib_conn = &iser_conn->ib_conn;
+		ib_dev = ib_conn->device->ib_device;
 		if (ib_conn->pi_support) {
-			u32 sig_caps = ib_conn->device->ib_device->attrs.sig_prot_cap;
+			u32 sig_caps = ib_dev->attrs.sig_prot_cap;
 
 			scsi_host_set_prot(shost, iser_dif_prot_caps(sig_caps));
 			scsi_host_set_guard(shost, SHOST_DIX_GUARD_IP |
 						   SHOST_DIX_GUARD_CRC);
 		}
 
-		if (iscsi_host_add(shost,
-				   ib_conn->device->ib_device->dev.parent)) {
+		if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
+			shost->virt_boundary_mask = ~MASK_4K;
+
+		if (iscsi_host_add(shost, ib_dev->dev.parent)) {
 			mutex_unlock(&iser_conn->state_mutex);
 			goto free_host;
 		}
@@ -958,30 +962,6 @@ static umode_t iser_attr_is_visible(int param_type, int param)
 	return 0;
 }
 
-static int iscsi_iser_slave_alloc(struct scsi_device *sdev)
-{
-	struct iscsi_session *session;
-	struct iser_conn *iser_conn;
-	struct ib_device *ib_dev;
-
-	mutex_lock(&unbind_iser_conn_mutex);
-
-	session = starget_to_session(scsi_target(sdev))->dd_data;
-	iser_conn = session->leadconn->dd_data;
-	if (!iser_conn) {
-		mutex_unlock(&unbind_iser_conn_mutex);
-		return -ENOTCONN;
-	}
-	ib_dev = iser_conn->ib_conn.device->ib_device;
-
-	if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
-		blk_queue_virt_boundary(sdev->request_queue, ~MASK_4K);
-
-	mutex_unlock(&unbind_iser_conn_mutex);
-
-	return 0;
-}
-
 static struct scsi_host_template iscsi_iser_sht = {
 	.module                 = THIS_MODULE,
 	.name                   = "iSCSI Initiator over iSER",
@@ -994,7 +974,6 @@ static struct scsi_host_template iscsi_iser_sht = {
 	.eh_device_reset_handler= iscsi_eh_device_reset,
 	.eh_target_reset_handler = iscsi_eh_recover_target,
 	.target_alloc		= iscsi_target_alloc,
-	.slave_alloc            = iscsi_iser_slave_alloc,
 	.proc_name              = "iscsi_iser",
 	.this_id                = -1,
 	.track_queue_depth	= 1,
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2019-06-17 12:19 ` [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host Christoph Hellwig
@ 2019-06-17 12:19 ` Christoph Hellwig
  2019-06-17 17:35   ` Sagi Grimberg
                     ` (2 more replies)
  2019-06-17 12:19 ` [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs Christoph Hellwig
                   ` (2 subsequent siblings)
  8 siblings, 3 replies; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

This ensures all proper DMA layer handling is taken care of by the
SCSI midlayer.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 4305da2c9037..b3a4ebd85046 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3063,20 +3063,6 @@ static int srp_target_alloc(struct scsi_target *starget)
 	return 0;
 }
 
-static int srp_slave_alloc(struct scsi_device *sdev)
-{
-	struct Scsi_Host *shost = sdev->host;
-	struct srp_target_port *target = host_to_target(shost);
-	struct srp_device *srp_dev = target->srp_host->srp_dev;
-	struct ib_device *ibdev = srp_dev->dev;
-
-	if (!(ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
-		blk_queue_virt_boundary(sdev->request_queue,
-					~srp_dev->mr_page_mask);
-
-	return 0;
-}
-
 static int srp_slave_configure(struct scsi_device *sdev)
 {
 	struct Scsi_Host *shost = sdev->host;
@@ -3279,7 +3265,6 @@ static struct scsi_host_template srp_template = {
 	.name				= "InfiniBand SRP initiator",
 	.proc_name			= DRV_NAME,
 	.target_alloc			= srp_target_alloc,
-	.slave_alloc			= srp_slave_alloc,
 	.slave_configure		= srp_slave_configure,
 	.info				= srp_target_info,
 	.queuecommand			= srp_queuecommand,
@@ -3814,6 +3799,9 @@ static ssize_t srp_create_target(struct device *dev,
 	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
 	target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
 
+	if (!(ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
+		target_host->virt_boundary_mask = ~srp_dev->mr_page_mask;
+
 	target = host_to_target(target_host);
 
 	target->net		= kobj_ns_grab_current(KOBJ_NS_TYPE_NET);
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2019-06-17 12:19 ` [PATCH 6/8] IB/srp: " Christoph Hellwig
@ 2019-06-17 12:19 ` Christoph Hellwig
  2019-06-18  0:46   ` Ming Lei
  2019-06-17 12:20 ` [PATCH 8/8] megaraid_sas: set an unlimited max_segment_size Christoph Hellwig
  2019-07-15 16:58 ` properly communicate queue limits to the DMA layer v2 Christoph Hellwig
  8 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:19 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

When using a virt_boundary_mask, as done for NVMe devices attached to
mpt3sas controllers we require an unlimited max_segment_size, as the
virt boundary merging code assumes that.  But we also need to propagate
that to the DMA mapping layer to make dma-debug happy.  The SCSI layer
takes care of that when using the per-host virt_boundary setting, but
given that mpt3sas only wants to set the virt_boundary for actual
NVMe devices we can't rely on that.  The DMA layer maximum segment
is global to the HBA however, so we have to set it explicitly.  This
patch assumes that mpt3sas does not have a segment size limitation,
which seems true based on the SGL format, but will need to be verified.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 1ccfbc7eebe0..c719b807f6d8 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -10222,6 +10222,7 @@ static struct scsi_host_template mpt3sas_driver_template = {
 	.this_id			= -1,
 	.sg_tablesize			= MPT3SAS_SG_DEPTH,
 	.max_sectors			= 32767,
+	.max_segment_size		= 0xffffffff,
 	.cmd_per_lun			= 7,
 	.shost_attrs			= mpt3sas_host_attrs,
 	.sdev_attrs			= mpt3sas_dev_attrs,
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 8/8] megaraid_sas: set an unlimited max_segment_size
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2019-06-17 12:19 ` [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs Christoph Hellwig
@ 2019-06-17 12:20 ` Christoph Hellwig
  2019-07-15 16:58 ` properly communicate queue limits to the DMA layer v2 Christoph Hellwig
  8 siblings, 0 replies; 32+ messages in thread
From: Christoph Hellwig @ 2019-06-17 12:20 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

When using a virt_boundary_mask, as done for NVMe devices attached to
megaraid_sas controllers we require an unlimited max_segment_size, as
the virt boundary merging code assumes that.  But we also need to
propagate that to the DMA mapping layer to make dma-debug happy.  The
SCSI layer takes care of that when using the per-host virt_boundary
setting, but given that megaraid_sas only wants to set the virt_boundary
for actual NVMe devices we can't rely on that.  The DMA layer maximum
segment is global to the HBA however, so we have to set it explicitly.
This patch assumes that megaraid_sas does not have a segment size
limitation, which seems true based on the SGL format, but will need
to be verified.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 3dd1df472dc6..59f709dbbab9 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3207,6 +3207,7 @@ static struct scsi_host_template megasas_template = {
 	.shost_attrs = megaraid_host_attrs,
 	.bios_param = megasas_bios_param,
 	.change_queue_depth = scsi_change_queue_depth,
+	.max_segment_size = 0xffffffff,
 	.no_write_same = 1,
 };
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host
  2019-06-17 12:19 ` [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host Christoph Hellwig
@ 2019-06-17 17:34   ` Sagi Grimberg
  2019-06-24 22:32     ` Max Gurtovoy
  1 sibling, 0 replies; 32+ messages in thread
From: Sagi Grimberg @ 2019-06-17 17:34 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Max Gurtovoy, Bart Van Assche, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

Acked-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host
  2019-06-17 12:19 ` [PATCH 6/8] IB/srp: " Christoph Hellwig
@ 2019-06-17 17:35   ` Sagi Grimberg
  2019-06-17 21:01   ` Bart Van Assche
  2019-06-24 22:33     ` Max Gurtovoy
  2 siblings, 0 replies; 32+ messages in thread
From: Sagi Grimberg @ 2019-06-17 17:35 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Max Gurtovoy, Bart Van Assche, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/8] scsi: add a host / host template field for the virt boundary
  2019-06-17 12:19 ` [PATCH 1/8] scsi: add a host / host template field for the virt boundary Christoph Hellwig
@ 2019-06-17 20:51   ` Bart Van Assche
  2019-06-18  0:35   ` Ming Lei
  1 sibling, 0 replies; 32+ messages in thread
From: Bart Van Assche @ 2019-06-17 20:51 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On 6/17/19 5:19 AM, Christoph Hellwig wrote:
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 65d0a10c76ad..d333bb6b1c59 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1775,7 +1775,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
>   	dma_set_seg_boundary(dev, shost->dma_boundary);
>   
>   	blk_queue_max_segment_size(q, shost->max_segment_size);
> -	dma_set_max_seg_size(dev, shost->max_segment_size);
> +	blk_queue_virt_boundary(q, shost->virt_boundary_mask);
> +	dma_set_max_seg_size(dev, queue_max_segment_size(q));

Although this looks fine to me for LLDs that own a PCIe device, I doubt 
this is correct for SCSI LLDs that share a PCIe device with other ULP 
drivers. From the RDMA core:

	/* Setup default max segment size for all IB devices */
	dma_set_max_seg_size(device->dma_device, SZ_2G);

Will instantiating a SCSI host (iSER or SRP) for an RDMA adapter cause 
the maximum segment size to be modified for all ULP drivers associated 
with that HCA?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/8] scsi: take the DMA max mapping size into account
  2019-06-17 12:19 ` [PATCH 2/8] scsi: take the DMA max mapping size into account Christoph Hellwig
@ 2019-06-17 20:56   ` Bart Van Assche
  2019-07-22  6:00     ` Ming Lei
  0 siblings, 1 reply; 32+ messages in thread
From: Bart Van Assche @ 2019-06-17 20:56 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On 6/17/19 5:19 AM, Christoph Hellwig wrote:
> We need to limit the devices max_sectors to what the DMA mapping
> implementation can support.  If not we risk running out of swiotlb
> buffers easily.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/scsi/scsi_lib.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index d333bb6b1c59..f233bfd84cd7 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1768,6 +1768,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
>   		blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
>   	}
>   
> +	shost->max_sectors = min_t(unsigned int, shost->max_sectors,
> +			dma_max_mapping_size(dev) << SECTOR_SHIFT);
>   	blk_queue_max_hw_sectors(q, shost->max_sectors);
>   	if (shost->unchecked_isa_dma)
>   		blk_queue_bounce_limit(q, BLK_BOUNCE_ISA);

Does dma_max_mapping_size() return a value in bytes? Is 
shost->max_sectors a number of sectors? If so, are you sure that "<< 
SECTOR_SHIFT" is the proper conversion? Shouldn't that be ">> 
SECTOR_SHIFT" instead?

Additionally, how about adding a comment above dma_max_mapping_size() 
that documents the unit of the returned number?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/8] ufshcd: set max_segment_size in the scsi host template
  2019-06-17 12:19 ` [PATCH 3/8] ufshcd: set max_segment_size in the scsi host template Christoph Hellwig
@ 2019-06-17 20:58   ` Bart Van Assche
  0 siblings, 0 replies; 32+ messages in thread
From: Bart Van Assche @ 2019-06-17 20:58 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On 6/17/19 5:19 AM, Christoph Hellwig wrote:
> We need to also mirror the value to the device to ensure IOMMU merging
> doesn't undo it, and the SCSI host level parameter will ensure that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/scsi/ufs/ufshcd.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 3fe3029617a8..505d625ed28d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -4587,8 +4587,6 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
>   	struct request_queue *q = sdev->request_queue;
>   
>   	blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
> -	blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
> -
>   	return 0;
>   }
>   
> @@ -6991,6 +6989,7 @@ static struct scsi_host_template ufshcd_driver_template = {
>   	.sg_tablesize		= SG_ALL,
>   	.cmd_per_lun		= UFSHCD_CMD_PER_LUN,
>   	.can_queue		= UFSHCD_CAN_QUEUE,
> +	.max_segment_size	= PRDT_DATA_BYTE_COUNT_MAX,
>   	.max_host_blocked	= 1,
>   	.track_queue_depth	= 1,
>   	.sdev_groups		= ufshcd_driver_groups,

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/8] storvsc: set virt_boundary_mask in the scsi host template
  2019-06-17 12:19 ` [PATCH 4/8] storvsc: set virt_boundary_mask " Christoph Hellwig
@ 2019-06-17 20:59   ` Bart Van Assche
  0 siblings, 0 replies; 32+ messages in thread
From: Bart Van Assche @ 2019-06-17 20:59 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On 6/17/19 5:19 AM, Christoph Hellwig wrote:
> This ensures all proper DMA layer handling is taken care of by the
> SCSI midlayer.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/scsi/storvsc_drv.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index b89269120a2d..7ed6f2fc1446 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1422,9 +1422,6 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
>   {
>   	blk_queue_rq_timeout(sdevice->request_queue, (storvsc_timeout * HZ));
>   
> -	/* Ensure there are no gaps in presented sgls */
> -	blk_queue_virt_boundary(sdevice->request_queue, PAGE_SIZE - 1);
> -
>   	sdevice->no_write_same = 1;
>   
>   	/*
> @@ -1697,6 +1694,8 @@ static struct scsi_host_template scsi_driver = {
>   	.this_id =		-1,
>   	/* Make sure we dont get a sg segment crosses a page boundary */
>   	.dma_boundary =		PAGE_SIZE-1,
> +	/* Ensure there are no gaps in presented sgls */
> +	.virt_boundary_mask =	PAGE_SIZE-1,
>   	.no_write_same =	1,
>   	.track_queue_depth =	1,
>   };

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host
  2019-06-17 12:19 ` [PATCH 6/8] IB/srp: " Christoph Hellwig
  2019-06-17 17:35   ` Sagi Grimberg
@ 2019-06-17 21:01   ` Bart Van Assche
  2019-06-24 22:33     ` Max Gurtovoy
  2 siblings, 0 replies; 32+ messages in thread
From: Bart Van Assche @ 2019-06-17 21:01 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On 6/17/19 5:19 AM, Christoph Hellwig wrote:
> This ensures all proper DMA layer handling is taken care of by the
> SCSI midlayer.

Acked-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/8] scsi: add a host / host template field for the virt boundary
  2019-06-17 12:19 ` [PATCH 1/8] scsi: add a host / host template field for the virt boundary Christoph Hellwig
  2019-06-17 20:51   ` Bart Van Assche
@ 2019-06-18  0:35   ` Ming Lei
  2019-06-20  6:17     ` Kashyap Desai
  1 sibling, 1 reply; 32+ messages in thread
From: Ming Lei @ 2019-06-18  0:35 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Martin K . Petersen, Sagi Grimberg, Max Gurtovoy,
	Bart Van Assche, linux-rdma, Linux SCSI List, megaraidlinux.pdl,
	MPT-FusionLinux.pdl, linux-hyperv, Linux Kernel Mailing List

On Mon, Jun 17, 2019 at 8:21 PM Christoph Hellwig <hch@lst.de> wrote:
>
> This allows drivers setting it up easily instead of branching out to
> block layer calls in slave_alloc, and ensures the upgraded
> max_segment_size setting gets picked up by the DMA layer.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/hosts.c     | 3 +++
>  drivers/scsi/scsi_lib.c  | 3 ++-
>  include/scsi/scsi_host.h | 3 +++
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index ff0d8c6a8d0c..55522b7162d3 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -462,6 +462,9 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>         else
>                 shost->dma_boundary = 0xffffffff;
>
> +       if (sht->virt_boundary_mask)
> +               shost->virt_boundary_mask = sht->virt_boundary_mask;
> +
>         device_initialize(&shost->shost_gendev);
>         dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
>         shost->shost_gendev.bus = &scsi_bus_type;
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 65d0a10c76ad..d333bb6b1c59 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1775,7 +1775,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
>         dma_set_seg_boundary(dev, shost->dma_boundary);
>
>         blk_queue_max_segment_size(q, shost->max_segment_size);
> -       dma_set_max_seg_size(dev, shost->max_segment_size);
> +       blk_queue_virt_boundary(q, shost->virt_boundary_mask);
> +       dma_set_max_seg_size(dev, queue_max_segment_size(q));

The patch looks fine, also suggest to make sure that max_segment_size
is block-size aligned, and un-aligned max segment size has caused trouble
on mmc.

Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs
  2019-06-17 12:19 ` [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs Christoph Hellwig
@ 2019-06-18  0:46   ` Ming Lei
  2019-06-20  7:04     ` Suganath Prabu Subramani
  0 siblings, 1 reply; 32+ messages in thread
From: Ming Lei @ 2019-06-18  0:46 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Martin K . Petersen, Sagi Grimberg, Max Gurtovoy,
	Bart Van Assche, linux-rdma, Linux SCSI List, megaraidlinux.pdl,
	MPT-FusionLinux.pdl, linux-hyperv, Linux Kernel Mailing List

On Mon, Jun 17, 2019 at 8:21 PM Christoph Hellwig <hch@lst.de> wrote:
>
> When using a virt_boundary_mask, as done for NVMe devices attached to
> mpt3sas controllers we require an unlimited max_segment_size, as the
> virt boundary merging code assumes that.  But we also need to propagate
> that to the DMA mapping layer to make dma-debug happy.  The SCSI layer
> takes care of that when using the per-host virt_boundary setting, but
> given that mpt3sas only wants to set the virt_boundary for actual
> NVMe devices we can't rely on that.  The DMA layer maximum segment
> is global to the HBA however, so we have to set it explicitly.  This
> patch assumes that mpt3sas does not have a segment size limitation,
> which seems true based on the SGL format, but will need to be verified.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 1ccfbc7eebe0..c719b807f6d8 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -10222,6 +10222,7 @@ static struct scsi_host_template mpt3sas_driver_template = {
>         .this_id                        = -1,
>         .sg_tablesize                   = MPT3SAS_SG_DEPTH,
>         .max_sectors                    = 32767,
> +       .max_segment_size               = 0xffffffff,

.max_segment_size should be aligned, either setting it here correctly or
forcing to make it aligned in scsi-core.

Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [PATCH 1/8] scsi: add a host / host template field for the virt boundary
  2019-06-18  0:35   ` Ming Lei
@ 2019-06-20  6:17     ` Kashyap Desai
  0 siblings, 0 replies; 32+ messages in thread
From: Kashyap Desai @ 2019-06-20  6:17 UTC (permalink / raw)
  To: Ming Lei, Christoph Hellwig
  Cc: Martin K . Petersen, Sagi Grimberg, Max Gurtovoy,
	Bart Van Assche, linux-rdma, Linux SCSI List, PDL,MEGARAIDLINUX,
	PDL-MPT-FUSIONLINUX, linux-hyperv, Linux Kernel Mailing List,
	Kashyap Desai

> -----Original Message-----
> From: megaraidlinux.pdl@broadcom.com
> [mailto:megaraidlinux.pdl@broadcom.com] On Behalf Of Ming Lei
> Sent: Tuesday, June 18, 2019 6:05 AM
> To: Christoph Hellwig <hch@lst.de>
> Cc: Martin K . Petersen <martin.petersen@oracle.com>; Sagi Grimberg
> <sagi@grimberg.me>; Max Gurtovoy <maxg@mellanox.com>; Bart Van
> Assche <bvanassche@acm.org>; linux-rdma <linux-rdma@vger.kernel.org>;
> Linux SCSI List <linux-scsi@vger.kernel.org>;
> megaraidlinux.pdl@broadcom.com; MPT-FusionLinux.pdl@broadcom.com;
> linux-hyperv@vger.kernel.org; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/8] scsi: add a host / host template field for the
> virt
> boundary
>
> On Mon, Jun 17, 2019 at 8:21 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > This allows drivers setting it up easily instead of branching out to
> > block layer calls in slave_alloc, and ensures the upgraded
> > max_segment_size setting gets picked up by the DMA layer.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  drivers/scsi/hosts.c     | 3 +++
> >  drivers/scsi/scsi_lib.c  | 3 ++-
> >  include/scsi/scsi_host.h | 3 +++
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index
> > ff0d8c6a8d0c..55522b7162d3 100644
> > --- a/drivers/scsi/hosts.c
> > +++ b/drivers/scsi/hosts.c
> > @@ -462,6 +462,9 @@ struct Scsi_Host *scsi_host_alloc(struct
> scsi_host_template *sht, int privsize)
> >         else
> >                 shost->dma_boundary = 0xffffffff;
> >
> > +       if (sht->virt_boundary_mask)
> > +               shost->virt_boundary_mask = sht->virt_boundary_mask;
> > +
> >         device_initialize(&shost->shost_gendev);
> >         dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
> >         shost->shost_gendev.bus = &scsi_bus_type; diff --git
> > a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index
> > 65d0a10c76ad..d333bb6b1c59 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -1775,7 +1775,8 @@ void __scsi_init_queue(struct Scsi_Host *shost,
> struct request_queue *q)
> >         dma_set_seg_boundary(dev, shost->dma_boundary);
> >
> >         blk_queue_max_segment_size(q, shost->max_segment_size);
> > -       dma_set_max_seg_size(dev, shost->max_segment_size);
> > +       blk_queue_virt_boundary(q, shost->virt_boundary_mask);
> > +       dma_set_max_seg_size(dev, queue_max_segment_size(q));
>
> The patch looks fine, also suggest to make sure that max_segment_size is
> block-size aligned, and un-aligned max segment size has caused trouble on
> mmc.

I validated changes on latest and few older series controllers.
Post changes, I noticed max_segment_size is updated.
find /sys/ -name max_segment_size  |grep sdb |xargs grep -r .
/sys/devices/pci0000:3a/0000:3a:00.0/0000:3b:00.0/0000:3c:04.0/0000:40:00.0/0000:41:00.0/0000:42:00.0/host0/target0:2:12/0:2:12:0/block/sdb/queue/max_segment_size:4294967295

I verify that single SGE having 1MB transfer length is working fine.

Acked-by: Kashyap Desai < kashyap.desai@broadcom.com>

>
> Thanks,
> Ming Lei
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs
  2019-06-18  0:46   ` Ming Lei
@ 2019-06-20  7:04     ` Suganath Prabu Subramani
  2019-06-20  7:08       ` Suganath Prabu Subramani
  0 siblings, 1 reply; 32+ messages in thread
From: Suganath Prabu Subramani @ 2019-06-20  7:04 UTC (permalink / raw)
  To: Ming Lei
  Cc: Christoph Hellwig, Martin K . Petersen, Sagi Grimberg,
	Max Gurtovoy, Bart Van Assche, linux-rdma, Linux SCSI List,
	megaraidlinux.pdl, PDL-MPT-FUSIONLINUX, linux-hyperv,
	Linux Kernel Mailing List

Please consider this as Acked-by: Suganath Prabu
<suganath-prabu.subramani@broadcom.com>


On Tue, Jun 18, 2019 at 6:16 AM Ming Lei <tom.leiming@gmail.com> wrote:
>
> On Mon, Jun 17, 2019 at 8:21 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > When using a virt_boundary_mask, as done for NVMe devices attached to
> > mpt3sas controllers we require an unlimited max_segment_size, as the
> > virt boundary merging code assumes that.  But we also need to propagate
> > that to the DMA mapping layer to make dma-debug happy.  The SCSI layer
> > takes care of that when using the per-host virt_boundary setting, but
> > given that mpt3sas only wants to set the virt_boundary for actual
> > NVMe devices we can't rely on that.  The DMA layer maximum segment
> > is global to the HBA however, so we have to set it explicitly.  This
> > patch assumes that mpt3sas does not have a segment size limitation,
> > which seems true based on the SGL format, but will need to be verified.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > index 1ccfbc7eebe0..c719b807f6d8 100644
> > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > @@ -10222,6 +10222,7 @@ static struct scsi_host_template mpt3sas_driver_template = {
> >         .this_id                        = -1,
> >         .sg_tablesize                   = MPT3SAS_SG_DEPTH,
> >         .max_sectors                    = 32767,
> > +       .max_segment_size               = 0xffffffff,
>
> .max_segment_size should be aligned, either setting it here correctly or
> forcing to make it aligned in scsi-core.
>
> Thanks,
> Ming Lei

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs
  2019-06-20  7:04     ` Suganath Prabu Subramani
@ 2019-06-20  7:08       ` Suganath Prabu Subramani
  0 siblings, 0 replies; 32+ messages in thread
From: Suganath Prabu Subramani @ 2019-06-20  7:08 UTC (permalink / raw)
  To: Ming Lei
  Cc: Christoph Hellwig, Martin K . Petersen, Sagi Grimberg,
	Max Gurtovoy, Bart Van Assche, linux-rdma, Linux SCSI List,
	megaraidlinux.pdl, PDL-MPT-FUSIONLINUX, linux-hyperv,
	Linux Kernel Mailing List

Acked-by: Suganath Prabu <suganath-prabu.subramani@broadcom.com>

On Thu, Jun 20, 2019 at 12:34 PM Suganath Prabu Subramani
<suganath-prabu.subramani@broadcom.com> wrote:
>
> Please consider this as Acked-by: Suganath Prabu
> <suganath-prabu.subramani@broadcom.com>
>
>
> On Tue, Jun 18, 2019 at 6:16 AM Ming Lei <tom.leiming@gmail.com> wrote:
> >
> > On Mon, Jun 17, 2019 at 8:21 PM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > When using a virt_boundary_mask, as done for NVMe devices attached to
> > > mpt3sas controllers we require an unlimited max_segment_size, as the
> > > virt boundary merging code assumes that.  But we also need to propagate
> > > that to the DMA mapping layer to make dma-debug happy.  The SCSI layer
> > > takes care of that when using the per-host virt_boundary setting, but
> > > given that mpt3sas only wants to set the virt_boundary for actual
> > > NVMe devices we can't rely on that.  The DMA layer maximum segment
> > > is global to the HBA however, so we have to set it explicitly.  This
> > > patch assumes that mpt3sas does not have a segment size limitation,
> > > which seems true based on the SGL format, but will need to be verified.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > > index 1ccfbc7eebe0..c719b807f6d8 100644
> > > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > > @@ -10222,6 +10222,7 @@ static struct scsi_host_template mpt3sas_driver_template = {
> > >         .this_id                        = -1,
> > >         .sg_tablesize                   = MPT3SAS_SG_DEPTH,
> > >         .max_sectors                    = 32767,
> > > +       .max_segment_size               = 0xffffffff,
> >
> > .max_segment_size should be aligned, either setting it here correctly or
> > forcing to make it aligned in scsi-core.
> >
> > Thanks,
> > Ming Lei

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host
  2019-06-17 12:19 ` [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host Christoph Hellwig
@ 2019-06-24 22:32     ` Max Gurtovoy
  2019-06-24 22:32     ` Max Gurtovoy
  1 sibling, 0 replies; 32+ messages in thread
From: Max Gurtovoy @ 2019-06-24 22:32 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Bart Van Assche, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On 6/17/2019 3:19 PM, Christoph Hellwig wrote:
> This ensures all proper DMA layer handling is taken care of by the
> SCSI midlayer.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>


Looks good,

Reviewed-by: Max Gurtovoy <maxg@mellanox.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host
@ 2019-06-24 22:32     ` Max Gurtovoy
  0 siblings, 0 replies; 32+ messages in thread
From: Max Gurtovoy @ 2019-06-24 22:32 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Bart Van Assche, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On 6/17/2019 3:19 PM, Christoph Hellwig wrote:
> This ensures all proper DMA layer handling is taken care of by the
> SCSI midlayer.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>


Looks good,

Reviewed-by: Max Gurtovoy <maxg@mellanox.com>


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host
  2019-06-17 12:19 ` [PATCH 6/8] IB/srp: " Christoph Hellwig
@ 2019-06-24 22:33     ` Max Gurtovoy
  2019-06-17 21:01   ` Bart Van Assche
  2019-06-24 22:33     ` Max Gurtovoy
  2 siblings, 0 replies; 32+ messages in thread
From: Max Gurtovoy @ 2019-06-24 22:33 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Bart Van Assche, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel


On 6/17/2019 3:19 PM, Christoph Hellwig wrote:
> This ensures all proper DMA layer handling is taken care of by the
> SCSI midlayer.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good,

Reviewed-by: Max Gurtovoy <maxg@mellanox.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host
@ 2019-06-24 22:33     ` Max Gurtovoy
  0 siblings, 0 replies; 32+ messages in thread
From: Max Gurtovoy @ 2019-06-24 22:33 UTC (permalink / raw)
  To: Christoph Hellwig, Martin K . Petersen
  Cc: Sagi Grimberg, Bart Van Assche, linux-rdma, linux-scsi,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel


On 6/17/2019 3:19 PM, Christoph Hellwig wrote:
> This ensures all proper DMA layer handling is taken care of by the
> SCSI midlayer.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good,

Reviewed-by: Max Gurtovoy <maxg@mellanox.com>


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: properly communicate queue limits to the DMA layer v2
  2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2019-06-17 12:20 ` [PATCH 8/8] megaraid_sas: set an unlimited max_segment_size Christoph Hellwig
@ 2019-07-15 16:58 ` Christoph Hellwig
  2019-07-15 17:33   ` Martin K. Petersen
  8 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2019-07-15 16:58 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: Sagi Grimberg, Max Gurtovoy, Bart Van Assche, linux-rdma,
	linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	linux-kernel

On Mon, Jun 17, 2019 at 02:19:52PM +0200, Christoph Hellwig wrote:
> Hi Martin,
> 
> we've always had a bit of a problem communicating the block layer
> queue limits to the DMA layer, which needs to respect them when
> an IOMMU that could merge segments is used.  Unfortunately most
> drivers don't get this right.  Oddly enough we've been mostly
> getting away with it, although lately dma-debug has been catching
> a few of those issues.

Ping?  What happened to this set of bug fixes?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: properly communicate queue limits to the DMA layer v2
  2019-07-15 16:58 ` properly communicate queue limits to the DMA layer v2 Christoph Hellwig
@ 2019-07-15 17:33   ` Martin K. Petersen
  2019-07-15 17:46     ` Christoph Hellwig
  0 siblings, 1 reply; 32+ messages in thread
From: Martin K. Petersen @ 2019-07-15 17:33 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Martin K . Petersen, Sagi Grimberg, Max Gurtovoy,
	Bart Van Assche, linux-rdma, linux-scsi, megaraidlinux.pdl,
	MPT-FusionLinux.pdl, linux-hyperv, linux-kernel


Christoph,

> Ping?  What happened to this set of bug fixes?

I thought they depended on Jens' tree?

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: properly communicate queue limits to the DMA layer v2
  2019-07-15 17:33   ` Martin K. Petersen
@ 2019-07-15 17:46     ` Christoph Hellwig
  2019-07-17  3:07       ` Martin K. Petersen
  0 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2019-07-15 17:46 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Christoph Hellwig, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
	linux-rdma, linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl,
	linux-hyperv, linux-kernel

On Mon, Jul 15, 2019 at 01:33:11PM -0400, Martin K. Petersen wrote:
> 
> Christoph,
> 
> > Ping?  What happened to this set of bug fixes?
> 
> I thought they depended on Jens' tree?

I think all the patches on the block side went into 5.2, but it's been
a while, so I might misremember..

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: properly communicate queue limits to the DMA layer v2
  2019-07-15 17:46     ` Christoph Hellwig
@ 2019-07-17  3:07       ` Martin K. Petersen
  0 siblings, 0 replies; 32+ messages in thread
From: Martin K. Petersen @ 2019-07-17  3:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Martin K. Petersen, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
	linux-rdma, linux-scsi, megaraidlinux.pdl, MPT-FusionLinux.pdl,
	linux-hyperv, linux-kernel


Christoph,

> I think all the patches on the block side went into 5.2, but it's been
> a while, so I might misremember..

I checked my notes and the reason I held them back was that I was
waiting for a response from Broadcom wrt. the megaraid segment size
limitation.  However, given that mpt3sas was acked, I assume it's the
same thing.

I'm not so keen on how big the last batch of patches for the merge
window is getting. But I queued your fixes up for 5.3.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/8] scsi: take the DMA max mapping size into account
  2019-06-17 20:56   ` Bart Van Assche
@ 2019-07-22  6:00     ` Ming Lei
  2019-07-22  6:18       ` Dexuan-Linux Cui
  2019-07-22  7:40       ` Damien Le Moal
  0 siblings, 2 replies; 32+ messages in thread
From: Ming Lei @ 2019-07-22  6:00 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Christoph Hellwig, Martin K . Petersen, Sagi Grimberg,
	Max Gurtovoy, linux-rdma, Linux SCSI List, megaraidlinux.pdl,
	MPT-FusionLinux.pdl, linux-hyperv, Linux Kernel Mailing List

On Tue, Jun 18, 2019 at 4:57 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 6/17/19 5:19 AM, Christoph Hellwig wrote:
> > We need to limit the devices max_sectors to what the DMA mapping
> > implementation can support.  If not we risk running out of swiotlb
> > buffers easily.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >   drivers/scsi/scsi_lib.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > index d333bb6b1c59..f233bfd84cd7 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -1768,6 +1768,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
> >               blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
> >       }
> >
> > +     shost->max_sectors = min_t(unsigned int, shost->max_sectors,
> > +                     dma_max_mapping_size(dev) << SECTOR_SHIFT);
> >       blk_queue_max_hw_sectors(q, shost->max_sectors);
> >       if (shost->unchecked_isa_dma)
> >               blk_queue_bounce_limit(q, BLK_BOUNCE_ISA);
>
> Does dma_max_mapping_size() return a value in bytes? Is
> shost->max_sectors a number of sectors? If so, are you sure that "<<
> SECTOR_SHIFT" is the proper conversion? Shouldn't that be ">>
> SECTOR_SHIFT" instead?

Now the patch has been committed, '<< SECTOR_SHIFT' needs to be fixed.

Also the following kernel oops is triggered on qemu, and looks
device->dma_mask is NULL.

[    5.826483] scsi host0: Virtio SCSI HBA
[    5.829302] st: Version 20160209, fixed bufsize 32768, s/g segs 256
[    5.831042] SCSI Media Changer driver v0.25
[    5.832491] ==================================================================
[    5.833332] BUG: KASAN: null-ptr-deref in
dma_direct_max_mapping_size+0x30/0x94
[    5.833332] Read of size 8 at addr 0000000000000000 by task kworker/u17:0/7
[    5.835506] nvme nvme0: pci function 0000:00:07.0
[    5.833332]
[    5.833332] CPU: 2 PID: 7 Comm: kworker/u17:0 Not tainted 5.3.0-rc1 #1328
[    5.836999] ahci 0000:00:1f.2: version 3.0
[    5.833332] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS ?-20180724_192412-buildhw-07.phx4
[    5.833332] Workqueue: events_unbound async_run_entry_fn
[    5.833332] Call Trace:
[    5.833332]  dump_stack+0x6f/0x9d
[    5.833332]  ? dma_direct_max_mapping_size+0x30/0x94
[    5.833332]  __kasan_report+0x161/0x189
[    5.833332]  ? dma_direct_max_mapping_size+0x30/0x94
[    5.833332]  kasan_report+0xe/0x12
[    5.833332]  dma_direct_max_mapping_size+0x30/0x94
[    5.833332]  __scsi_init_queue+0xd8/0x1f3
[    5.833332]  scsi_mq_alloc_queue+0x62/0x89
[    5.833332]  scsi_alloc_sdev+0x38c/0x479
[    5.833332]  scsi_probe_and_add_lun+0x22d/0x1093
[    5.833332]  ? kobject_set_name_vargs+0xa4/0xb2
[    5.833332]  ? mutex_lock+0x88/0xc4
[    5.833332]  ? scsi_free_host_dev+0x4a/0x4a
[    5.833332]  ? _raw_spin_lock_irqsave+0x8c/0xde
[    5.833332]  ? _raw_write_unlock_irqrestore+0x23/0x23
[    5.833332]  ? ata_tdev_match+0x22/0x45
[    5.833332]  ? attribute_container_add_device+0x160/0x17e
[    5.833332]  ? rpm_resume+0x26a/0x7c0
[    5.833332]  ? kobject_get+0x12/0x43
[    5.833332]  ? rpm_put_suppliers+0x7e/0x7e
[    5.833332]  ? _raw_spin_lock_irqsave+0x8c/0xde
[    5.833332]  ? _raw_write_unlock_irqrestore+0x23/0x23
[    5.833332]  ? scsi_target_destroy+0x135/0x135
[    5.833332]  __scsi_scan_target+0x14b/0x6aa
[    5.833332]  ? pvclock_clocksource_read+0xc0/0x14e
[    5.833332]  ? scsi_add_device+0x20/0x20
[    5.833332]  ? rpm_resume+0x1ae/0x7c0
[    5.833332]  ? rpm_put_suppliers+0x7e/0x7e
[    5.833332]  ? _raw_spin_lock_irqsave+0x8c/0xde
[    5.833332]  ? _raw_write_unlock_irqrestore+0x23/0x23
[    5.833332]  ? pick_next_task_fair+0x976/0xa3d
[    5.833332]  ? mutex_lock+0x88/0xc4
[    5.833332]  scsi_scan_channel+0x76/0x9e
[    5.833332]  scsi_scan_host_selected+0x131/0x176
[    5.833332]  ? scsi_scan_host+0x241/0x241
[    5.833332]  do_scan_async+0x27/0x219
[    5.833332]  ? scsi_scan_host+0x241/0x241
[    5.833332]  async_run_entry_fn+0xdc/0x23d
[    5.833332]  process_one_work+0x327/0x539
[    5.833332]  worker_thread+0x330/0x492
[    5.833332]  ? rescuer_thread+0x41f/0x41f
[    5.833332]  kthread+0x1c6/0x1d5
[    5.833332]  ? kthread_park+0xd3/0xd3
[    5.833332]  ret_from_fork+0x1f/0x30
[    5.833332] ==================================================================



Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/8] scsi: take the DMA max mapping size into account
  2019-07-22  6:00     ` Ming Lei
@ 2019-07-22  6:18       ` Dexuan-Linux Cui
  2019-07-22  7:40       ` Damien Le Moal
  1 sibling, 0 replies; 32+ messages in thread
From: Dexuan-Linux Cui @ 2019-07-22  6:18 UTC (permalink / raw)
  To: Ming Lei
  Cc: Bart Van Assche, Christoph Hellwig, Martin K . Petersen,
	Sagi Grimberg, Max Gurtovoy, linux-rdma, Linux SCSI List,
	megaraidlinux.pdl, MPT-FusionLinux.pdl, linux-hyperv,
	Linux Kernel Mailing List, Dexuan Cui, v-lide

On Sun, Jul 21, 2019 at 11:01 PM Ming Lei <tom.leiming@gmail.com> wrote:
>
> On Tue, Jun 18, 2019 at 4:57 AM Bart Van Assche <bvanassche@acm.org> wrote:
> >
> > On 6/17/19 5:19 AM, Christoph Hellwig wrote:
> > > We need to limit the devices max_sectors to what the DMA mapping
> > > implementation can support.  If not we risk running out of swiotlb
> > > buffers easily.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >   drivers/scsi/scsi_lib.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > > index d333bb6b1c59..f233bfd84cd7 100644
> > > --- a/drivers/scsi/scsi_lib.c
> > > +++ b/drivers/scsi/scsi_lib.c
> > > @@ -1768,6 +1768,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
> > >               blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
> > >       }
> > >
> > > +     shost->max_sectors = min_t(unsigned int, shost->max_sectors,
> > > +                     dma_max_mapping_size(dev) << SECTOR_SHIFT);
> > >       blk_queue_max_hw_sectors(q, shost->max_sectors);
> > >       if (shost->unchecked_isa_dma)
> > >               blk_queue_bounce_limit(q, BLK_BOUNCE_ISA);
> >
> > Does dma_max_mapping_size() return a value in bytes? Is
> > shost->max_sectors a number of sectors? If so, are you sure that "<<
> > SECTOR_SHIFT" is the proper conversion? Shouldn't that be ">>
> > SECTOR_SHIFT" instead?
>
> Now the patch has been committed, '<< SECTOR_SHIFT' needs to be fixed.
>
> Also the following kernel oops is triggered on qemu, and looks
> device->dma_mask is NULL.
>
> Ming Lei

FYI: we also see the panic with a Linux kernel 5.2.0-next-20190719
running on Hyper-V:

[    7.429053] RIP: 0010:dma_direct_max_mapping_size+0x26/0x80
[    7.429053] Code: 0f b6 c0 c3 0f 1f 44 00 00 55 48 89 e5 41 54 53
48 89 fb e8 4c 14 00 00 84 c0 74 45 48 8b 83 28 02 00 00 4c 8b a3 38
02 00 00 <48> 8b 00 48 85 c0 74 0c 4d 85 e4 74 36 49 39 c4 4c 0f 47 e0
48 89
[    7.429053] RSP: 0018:ffffc1d5005efbc0 EFLAGS: 00010202
[    7.429053] RAX: 0000000000000000 RBX: ffff9cf86d24c428 RCX: 0000000000000000
[    7.429053] RDX: ffff9cf86d12dd00 RSI: 0000000000000200 RDI: ffff9cf86d24c428
[    7.429053] RBP: ffffc1d5005efbd0 R08: ffff9cf86fcaf0e0 R09: ffff9cf86e0072c0
[    7.429053] R10: ffffc1d5005efa70 R11: 00000000000301a0 R12: 0000000000000000
[    7.429053] R13: ffff9cf86d24c428 R14: 0000000000000400 R15: ffff9cf825cff000
[    7.429053] FS:  0000000000000000(0000) GS:ffff9cf86fc80000(0000)
knlGS:0000000000000000
[    7.429053] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    7.429053] CR2: 0000000000000000 CR3: 00000003c700a001 CR4: 00000000003606e0
[    7.456569] NET: Registered protocol family 17
[    7.429053] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    7.469803] Key type dns_resolver registered
[    7.429053] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    7.429053] Call Trace:
[    7.429053]  dma_max_mapping_size+0x39/0x50
[    7.429053]  __scsi_init_queue+0x7f/0x140
[    7.429053]  scsi_mq_alloc_queue+0x38/0x60
[    7.429053]  scsi_alloc_sdev+0x1da/0x2b0
[    7.429053]  scsi_probe_and_add_lun+0x471/0xe60
[    7.429053]  __scsi_scan_target+0xfc/0x610
[    7.429053]  scsi_scan_channel+0x66/0xa0
[    7.429053]  scsi_scan_host_selected+0xf3/0x160
[    7.429053]  do_scsi_scan_host+0x93/0xa0
[    7.429053]  do_scan_async+0x1c/0x190
[    7.429053]  async_run_entry_fn+0x3c/0x150
[    7.429053]  process_one_work+0x1f7/0x3f0
[    7.429053]  worker_thread+0x34/0x400
[    7.429053]  kthread+0x121/0x140
[    7.429053]  ret_from_fork+0x35/0x40
[    7.429053] Modules linked in:
[    7.429053] CR2: 0000000000000000
[    7.766122] BUG: kernel NULL pointer dereference, address: 0000000000000000

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/8] scsi: take the DMA max mapping size into account
  2019-07-22  6:00     ` Ming Lei
  2019-07-22  6:18       ` Dexuan-Linux Cui
@ 2019-07-22  7:40       ` Damien Le Moal
  1 sibling, 0 replies; 32+ messages in thread
From: Damien Le Moal @ 2019-07-22  7:40 UTC (permalink / raw)
  To: Ming Lei, Bart van Assche
  Cc: Christoph Hellwig, Martin K . Petersen, Sagi Grimberg,
	Max Gurtovoy, linux-rdma, Linux SCSI List, megaraidlinux.pdl,
	MPT-FusionLinux.pdl, linux-hyperv, Linux Kernel Mailing List

On 2019/07/22 15:01, Ming Lei wrote:
> On Tue, Jun 18, 2019 at 4:57 AM Bart Van Assche <bvanassche@acm.org> wrote:
>>
>> On 6/17/19 5:19 AM, Christoph Hellwig wrote:
>>> We need to limit the devices max_sectors to what the DMA mapping
>>> implementation can support.  If not we risk running out of swiotlb
>>> buffers easily.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>> ---
>>>   drivers/scsi/scsi_lib.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>>> index d333bb6b1c59..f233bfd84cd7 100644
>>> --- a/drivers/scsi/scsi_lib.c
>>> +++ b/drivers/scsi/scsi_lib.c
>>> @@ -1768,6 +1768,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
>>>               blk_queue_max_integrity_segments(q, shost->sg_prot_tablesize);
>>>       }
>>>
>>> +     shost->max_sectors = min_t(unsigned int, shost->max_sectors,
>>> +                     dma_max_mapping_size(dev) << SECTOR_SHIFT);
>>>       blk_queue_max_hw_sectors(q, shost->max_sectors);
>>>       if (shost->unchecked_isa_dma)
>>>               blk_queue_bounce_limit(q, BLK_BOUNCE_ISA);
>>
>> Does dma_max_mapping_size() return a value in bytes? Is
>> shost->max_sectors a number of sectors? If so, are you sure that "<<
>> SECTOR_SHIFT" is the proper conversion? Shouldn't that be ">>
>> SECTOR_SHIFT" instead?
> 
> Now the patch has been committed, '<< SECTOR_SHIFT' needs to be fixed.
> 
> Also the following kernel oops is triggered on qemu, and looks
> device->dma_mask is NULL.

Just hit the exact same problem using tcmu-runner (ZBC file handler) on bare
metal (no QEMU). dev->dma_mask is NULL. No problem with real disks though.

> 
> [    5.826483] scsi host0: Virtio SCSI HBA
> [    5.829302] st: Version 20160209, fixed bufsize 32768, s/g segs 256
> [    5.831042] SCSI Media Changer driver v0.25
> [    5.832491] ==================================================================
> [    5.833332] BUG: KASAN: null-ptr-deref in
> dma_direct_max_mapping_size+0x30/0x94
> [    5.833332] Read of size 8 at addr 0000000000000000 by task kworker/u17:0/7
> [    5.835506] nvme nvme0: pci function 0000:00:07.0
> [    5.833332]
> [    5.833332] CPU: 2 PID: 7 Comm: kworker/u17:0 Not tainted 5.3.0-rc1 #1328
> [    5.836999] ahci 0000:00:1f.2: version 3.0
> [    5.833332] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
> BIOS ?-20180724_192412-buildhw-07.phx4
> [    5.833332] Workqueue: events_unbound async_run_entry_fn
> [    5.833332] Call Trace:
> [    5.833332]  dump_stack+0x6f/0x9d
> [    5.833332]  ? dma_direct_max_mapping_size+0x30/0x94
> [    5.833332]  __kasan_report+0x161/0x189
> [    5.833332]  ? dma_direct_max_mapping_size+0x30/0x94
> [    5.833332]  kasan_report+0xe/0x12
> [    5.833332]  dma_direct_max_mapping_size+0x30/0x94
> [    5.833332]  __scsi_init_queue+0xd8/0x1f3
> [    5.833332]  scsi_mq_alloc_queue+0x62/0x89
> [    5.833332]  scsi_alloc_sdev+0x38c/0x479
> [    5.833332]  scsi_probe_and_add_lun+0x22d/0x1093
> [    5.833332]  ? kobject_set_name_vargs+0xa4/0xb2
> [    5.833332]  ? mutex_lock+0x88/0xc4
> [    5.833332]  ? scsi_free_host_dev+0x4a/0x4a
> [    5.833332]  ? _raw_spin_lock_irqsave+0x8c/0xde
> [    5.833332]  ? _raw_write_unlock_irqrestore+0x23/0x23
> [    5.833332]  ? ata_tdev_match+0x22/0x45
> [    5.833332]  ? attribute_container_add_device+0x160/0x17e
> [    5.833332]  ? rpm_resume+0x26a/0x7c0
> [    5.833332]  ? kobject_get+0x12/0x43
> [    5.833332]  ? rpm_put_suppliers+0x7e/0x7e
> [    5.833332]  ? _raw_spin_lock_irqsave+0x8c/0xde
> [    5.833332]  ? _raw_write_unlock_irqrestore+0x23/0x23
> [    5.833332]  ? scsi_target_destroy+0x135/0x135
> [    5.833332]  __scsi_scan_target+0x14b/0x6aa
> [    5.833332]  ? pvclock_clocksource_read+0xc0/0x14e
> [    5.833332]  ? scsi_add_device+0x20/0x20
> [    5.833332]  ? rpm_resume+0x1ae/0x7c0
> [    5.833332]  ? rpm_put_suppliers+0x7e/0x7e
> [    5.833332]  ? _raw_spin_lock_irqsave+0x8c/0xde
> [    5.833332]  ? _raw_write_unlock_irqrestore+0x23/0x23
> [    5.833332]  ? pick_next_task_fair+0x976/0xa3d
> [    5.833332]  ? mutex_lock+0x88/0xc4
> [    5.833332]  scsi_scan_channel+0x76/0x9e
> [    5.833332]  scsi_scan_host_selected+0x131/0x176
> [    5.833332]  ? scsi_scan_host+0x241/0x241
> [    5.833332]  do_scan_async+0x27/0x219
> [    5.833332]  ? scsi_scan_host+0x241/0x241
> [    5.833332]  async_run_entry_fn+0xdc/0x23d
> [    5.833332]  process_one_work+0x327/0x539
> [    5.833332]  worker_thread+0x330/0x492
> [    5.833332]  ? rescuer_thread+0x41f/0x41f
> [    5.833332]  kthread+0x1c6/0x1d5
> [    5.833332]  ? kthread_park+0xd3/0xd3
> [    5.833332]  ret_from_fork+0x1f/0x30
> [    5.833332] ==================================================================
> 
> 
> 
> Thanks,
> Ming Lei
> 


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2019-07-22  7:40 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17 12:19 properly communicate queue limits to the DMA layer v2 Christoph Hellwig
2019-06-17 12:19 ` [PATCH 1/8] scsi: add a host / host template field for the virt boundary Christoph Hellwig
2019-06-17 20:51   ` Bart Van Assche
2019-06-18  0:35   ` Ming Lei
2019-06-20  6:17     ` Kashyap Desai
2019-06-17 12:19 ` [PATCH 2/8] scsi: take the DMA max mapping size into account Christoph Hellwig
2019-06-17 20:56   ` Bart Van Assche
2019-07-22  6:00     ` Ming Lei
2019-07-22  6:18       ` Dexuan-Linux Cui
2019-07-22  7:40       ` Damien Le Moal
2019-06-17 12:19 ` [PATCH 3/8] ufshcd: set max_segment_size in the scsi host template Christoph Hellwig
2019-06-17 20:58   ` Bart Van Assche
2019-06-17 12:19 ` [PATCH 4/8] storvsc: set virt_boundary_mask " Christoph Hellwig
2019-06-17 20:59   ` Bart Van Assche
2019-06-17 12:19 ` [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host Christoph Hellwig
2019-06-17 17:34   ` Sagi Grimberg
2019-06-24 22:32   ` Max Gurtovoy
2019-06-24 22:32     ` Max Gurtovoy
2019-06-17 12:19 ` [PATCH 6/8] IB/srp: " Christoph Hellwig
2019-06-17 17:35   ` Sagi Grimberg
2019-06-17 21:01   ` Bart Van Assche
2019-06-24 22:33   ` Max Gurtovoy
2019-06-24 22:33     ` Max Gurtovoy
2019-06-17 12:19 ` [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs Christoph Hellwig
2019-06-18  0:46   ` Ming Lei
2019-06-20  7:04     ` Suganath Prabu Subramani
2019-06-20  7:08       ` Suganath Prabu Subramani
2019-06-17 12:20 ` [PATCH 8/8] megaraid_sas: set an unlimited max_segment_size Christoph Hellwig
2019-07-15 16:58 ` properly communicate queue limits to the DMA layer v2 Christoph Hellwig
2019-07-15 17:33   ` Martin K. Petersen
2019-07-15 17:46     ` Christoph Hellwig
2019-07-17  3:07       ` Martin K. Petersen

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.