Re: [PATCH 2/4] block: force an unlimited segment size on queues with a virt boundary

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Tue, 2019-05-21 at 09:01 +0200, Christoph Hellwig wrote:
> We currently fail to update the front/back segment size in the bio
> when
> deciding to allow an otherwise gappy segement to a device with a
> virt boundary.  The reason why this did not cause problems is that
> devices with a virt boundary fundamentally don't use segments as we
> know it and thus don't care.  Make that assumption formal by forcing
> an unlimited segement size in this case.
> 
> Fixes: f6970f83ef79 ("block: don't check if adjacent bvecs in one bio
> can be mergeable")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> ---
>  block/blk-settings.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 3facc41476be..2ae348c101a0 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -310,6 +310,9 @@ void blk_queue_max_segment_size(struct
> request_queue *q, unsigned int max_size)
>  		       __func__, max_size);
>  	}
>  
> +	/* see blk_queue_virt_boundary() for the explanation */
> +	WARN_ON_ONCE(q->limits.virt_boundary_mask);
> +
>  	q->limits.max_segment_size = max_size;
>  }
>  EXPORT_SYMBOL(blk_queue_max_segment_size);
> @@ -742,6 +745,14 @@ EXPORT_SYMBOL(blk_queue_segment_boundary);
>  void blk_queue_virt_boundary(struct request_queue *q, unsigned long
> mask)
>  {
>  	q->limits.virt_boundary_mask = mask;
> +
> +	/*
> +	 * Devices that require a virtual boundary do not support
> scatter/gather
> +	 * I/O natively, but instead require a descriptor list entry
> for each
> +	 * page (which might not be idential to the Linux
> PAGE_SIZE).  Because
> +	 * of that they are not limited by our notion of "segment
> size".
> +	 */
> +	q->limits.max_segment_size = UINT_MAX;

This needs to be conditional.  If you did set the max segment size, the
block layer should still respect it and not do an override here.  This
seems to be what's causing a regression in SCSI because we set the
max_segment_size first then call the blk_queue_virt_boundary. 
Additionally, if you're not setting the virt boundary (mask == 0) this
should not be set.  I suggest the below.

James

---
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 2ae348c101a0..46a95536f3bd 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -752,7 +752,8 @@ void blk_queue_virt_boundary(struct request_queue *q, unsigned long mask)
 	 * page (which might not be idential to the Linux PAGE_SIZE).  Because
 	 * of that they are not limited by our notion of "segment size".
 	 */
-	q->limits.max_segment_size = UINT_MAX;
+	if (mask != 0 && q->limits.max_segment_size == BLK_MAX_SEGMENT_SIZE)
+		q->limits.max_segment_size = UINT_MAX;
 }
 EXPORT_SYMBOL(blk_queue_virt_boundary);