properly communicate queue limits to the DMA layer v2

properly communicate queue limits to the DMA layer v2

[Christoph Hellwig]

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

[PATCH 1/8] scsi: add a host / host template field for the virt boundary

[Christoph Hellwig]

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

Re: [PATCH 1/8] scsi: add a host / host template field for the virt boundary

[Bart Van Assche]

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.

Re: [PATCH 1/8] scsi: add a host / host template field for the virt boundary

[Ming Lei]

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

RE: [PATCH 1/8] scsi: add a host / host template field for the virt boundary

[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
>

[PATCH 2/8] scsi: take the DMA max mapping size into account

[Christoph Hellwig]

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

Re: [PATCH 2/8] scsi: take the DMA max mapping size into account

[Bart Van Assche]

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.

Re: [PATCH 2/8] scsi: take the DMA max mapping size into account

[Ming Lei]

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

Re: [PATCH 2/8] scsi: take the DMA max mapping size into account

[Dexuan-Linux Cui]

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

Re: [PATCH 2/8] scsi: take the DMA max mapping size into account

[Damien Le Moal]

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

[PATCH 3/8] ufshcd: set max_segment_size in the scsi host template

[Christoph Hellwig]

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

Re: [PATCH 3/8] ufshcd: set max_segment_size in the scsi host template

[Bart Van Assche]

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>

[PATCH 4/8] storvsc: set virt_boundary_mask in the scsi host template

[Christoph Hellwig]

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

Re: [PATCH 4/8] storvsc: set virt_boundary_mask in the scsi host template

[Bart Van Assche]

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>

[PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host

[Christoph Hellwig]

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

Re: [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host

[Sagi Grimberg]

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

Re: [PATCH 5/8] IB/iser: set virt_boundary_mask in the scsi host

[Max Gurtovoy]

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>

[PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host

[Christoph Hellwig]

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

Re: [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host

[Sagi Grimberg]

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

Re: [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host

[Bart Van Assche]

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>

Re: [PATCH 6/8] IB/srp: set virt_boundary_mask in the scsi host

[Max Gurtovoy]

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>

[PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs

[Christoph Hellwig]

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

Re: [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs

[Ming Lei]

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

Re: [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs

[Suganath Prabu Subramani]

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

Re: [PATCH 7/8] mpt3sas: set an unlimited max_segment_size for SAS 3.0 HBAs

[Suganath Prabu Subramani]

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

[PATCH 8/8] megaraid_sas: set an unlimited max_segment_size

[Christoph Hellwig]

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

Re: properly communicate queue limits to the DMA layer v2

[Christoph Hellwig]

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?

Re: properly communicate queue limits to the DMA layer v2

[Martin K. Petersen]

Christoph,

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

I thought they depended on Jens' tree?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: properly communicate queue limits to the DMA layer v2

[Christoph Hellwig]

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..

Re: properly communicate queue limits to the DMA layer v2

[Martin K. Petersen]

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