blk-merge: BIO front merge size check for chunked devices

[Damien Le Moal]

Hello,

In ll_front_merge_fn, the request size result of the eventual front
merge of a BIO with a request is checked against the maximum
size allowed for the request using blk_rq_get_max_sectors. This
function will also check that the merge result will not span block
chunks for chunked block devices using the function blk_max_size_offset.

However, blk_rq_get_max_sectors always calls blk_max_size_offset using
the request sector position, which in the case of a front merge is not
the position at which the merged request would be issued: the sector
position to use must be that of the BIO, and not that of the request.

This problem can trigger a “boundary violation error” for write
requests on ZAC/ZBC host-managed SMR disks as the last write BIO
of a zone (a chunk) can end up being front-merged with the first
request of the following zone (chunk).

The attached patch against linux-4.5-rc2 fixes this problem by adding
an offset argument to the function blk_rq_get_max_sectors.

Best regards.

diff -Naur linux-4.5-rc2/block/blk-merge.c 
linux-4.5-rc2-patched/block/blk-merge.c
--- linux-4.5-rc2/block/blk-merge.c 2016-02-01 11:12:16.0 +0900
+++ linux-4.5-rc2-patched/block/blk-merge.c 2016-02-02 15:28:07.626107509 
+0900
@@ -504,7 +504,7 @@
integrity_req_gap_back_merge(req, bio))
return 0;
if (blk_rq_sectors(req) + bio_sectors(bio) >
-   blk_rq_get_max_sectors(req)) {
+   blk_rq_get_max_sectors(req, blk_rq_pos(req))) {
req->cmd_flags |= REQ_NOMERGE;
if (req == q->last_merge)
q->last_merge = NULL;
@@ -528,7 +528,7 @@
integrity_req_gap_front_merge(req, bio))
return 0;
if (blk_rq_sectors(req) + bio_sectors(bio) >
-   blk_rq_get_max_sectors(req)) {
+   blk_rq_get_max_sectors(req, bio->bi_iter.bi_sector)) {
req->cmd_flags |= REQ_NOMERGE;
if (req == q->last_merge)
q->last_merge = NULL;
@@ -574,7 +574,7 @@
 * Will it become too large?
 */
if ((blk_rq_sectors(req) + blk_rq_sectors(next)) >
-   blk_rq_get_max_sectors(req))
+   blk_rq_get_max_sectors(req, blk_rq_pos(req)))
return 0;
 
total_phys_segments = req->nr_phys_segments + next->nr_phys_segments;
diff -Naur linux-4.5-rc2/include/linux/blkdev.h 
linux-4.5-rc2-patched/include/linux/blkdev.h
--- linux-4.5-rc2/include/linux/blkdev.h2016-02-01 11:12:16.0 
+0900
+++ linux-4.5-rc2-patched/include/linux/blkdev.h2016-02-02 
15:26:39.313283902 +0900
@@ -888,7 +888,8 @@
(offset & (q->limits.chunk_sectors - 1));
 }
 
-static inline unsigned int blk_rq_get_max_sectors(struct request *rq)
+static inline unsigned int blk_rq_get_max_sectors(struct request *rq,
+  sector_t offset)
 {
struct request_queue *q = rq->q;
 
@@ -898,7 +899,7 @@
if (!q->limits.chunk_sectors || (rq->cmd_flags & REQ_DISCARD))
return blk_queue_get_max_sectors(q, rq->cmd_flags);
 
-   return min(blk_max_size_offset(q, blk_rq_pos(rq)),
+   return min(blk_max_size_offset(q, offset),
blk_queue_get_max_sectors(q, rq->cmd_flags));
 }


--
Damien Le Moal,
System Software Group, WW Research,
HGST, a Western Digital company
Western Digital Corporation (and its subsidiaries) E-mail Confidentiality 
Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or 
legally privileged information of WDC and/or its affiliates, and are intended 
solely for the use of the individual or entity to which they are addressed. If 
you are not the intended recipient, any disclosure, copying, distribution or 
any action taken or omitted to be taken in reliance on it, is prohibited. If 
you have received this e-mail in error, please notify the sender immediately 
and delete the e-mail in its entirety from your system.


bio-merge-size-check.patch
Description: bio-merge-size-check.patch

Re: blk-merge: BIO front merge size check for chunked devices

[Hannes Reinecke]

On 02/02/2016 08:06 AM, Damien Le Moal wrote:
> 
> Hello,
> 
> In ll_front_merge_fn, the request size result of the eventual front
> merge of a BIO with a request is checked against the maximum
> size allowed for the request using blk_rq_get_max_sectors. This
> function will also check that the merge result will not span block
> chunks for chunked block devices using the function blk_max_size_offset.
> 
> However, blk_rq_get_max_sectors always calls blk_max_size_offset using
> the request sector position, which in the case of a front merge is not
> the position at which the merged request would be issued: the sector
> position to use must be that of the BIO, and not that of the request.
> 
> This problem can trigger a “boundary violation error” for write
> requests on ZAC/ZBC host-managed SMR disks as the last write BIO
> of a zone (a chunk) can end up being front-merged with the first
> request of the following zone (chunk).
> 
> The attached patch against linux-4.5-rc2 fixes this problem by adding
> an offset argument to the function blk_rq_get_max_sectors.
> 

Reviewed-by: Hannes Reinecke 

I can easily respin this as a proper patch if required.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html