linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] block: fix regression where bio_integrity_process uses wrong bio_vec iterator
@ 2014-11-26  1:40 Darrick J. Wong
  2014-11-26  4:57 ` Martin K. Petersen
  0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2014-11-26  1:40 UTC (permalink / raw)
  To: Jens Axboe, Martin K. Petersen; +Cc: linux-kernel, linux-fsdevel

bio integrity handling is broken on a system with LVM layered atop a
DIF/DIX SCSI drive because device mapper clones the bio, modifies the
clone, and sends the clone to the lower layers for processing.
However, the clone bio has bi_vcnt == 0, which means that when the sd
driver calls bio_integrity_process to attach DIX data, the
for_each_segment_all() call (which uses bi_vcnt) returns immediately
and random garbage is sent to the disk on a disk write.  The disk of
course returns an error.

Therefore, teach bio_integrity_process() to use bio_for_each_segment()
to iterate the bio_vecs, since the per-bio iterator tracks which
bio_vecs are associated with that particular bio.  The integrity
handling code is effectively part of the "driver" (it's not the bio
owner), so it must use the correct iterator function.

v2: Fix a compiler warning about abandoned local variables.  This
patch supersedes "block: bio_integrity_process uses wrong bio_vec
iterator".  Patch applies against 3.18-rc6.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 block/bio-integrity.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 0984232..5cbd5d9 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -216,9 +216,10 @@ static int bio_integrity_process(struct bio *bio,
 {
 	struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev);
 	struct blk_integrity_iter iter;
-	struct bio_vec *bv;
+	struct bvec_iter bviter;
+	struct bio_vec bv;
 	struct bio_integrity_payload *bip = bio_integrity(bio);
-	unsigned int i, ret = 0;
+	unsigned int ret = 0;
 	void *prot_buf = page_address(bip->bip_vec->bv_page) +
 		bip->bip_vec->bv_offset;
 
@@ -227,11 +228,11 @@ static int bio_integrity_process(struct bio *bio,
 	iter.seed = bip_get_seed(bip);
 	iter.prot_buf = prot_buf;
 
-	bio_for_each_segment_all(bv, bio, i) {
-		void *kaddr = kmap_atomic(bv->bv_page);
+	bio_for_each_segment(bv, bio, bviter) {
+		void *kaddr = kmap_atomic(bv.bv_page);
 
-		iter.data_buf = kaddr + bv->bv_offset;
-		iter.data_size = bv->bv_len;
+		iter.data_buf = kaddr + bv.bv_offset;
+		iter.data_size = bv.bv_len;
 
 		ret = proc_fn(&iter);
 		if (ret) {

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

* Re: [PATCH v2] block: fix regression where bio_integrity_process uses wrong bio_vec iterator
  2014-11-26  1:40 [PATCH v2] block: fix regression where bio_integrity_process uses wrong bio_vec iterator Darrick J. Wong
@ 2014-11-26  4:57 ` Martin K. Petersen
  2014-12-02 15:15   ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Martin K. Petersen @ 2014-11-26  4:57 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Jens Axboe, Martin K. Petersen, linux-kernel, linux-fsdevel

>>>>> "Darrick" == Darrick J Wong <darrick.wong@oracle.com> writes:

Darrick> Therefore, teach bio_integrity_process() to use
Darrick> bio_for_each_segment() to iterate the bio_vecs, since the
Darrick> per-bio iterator tracks which bio_vecs are associated with that
Darrick> particular bio.  The integrity handling code is effectively
Darrick> part of the "driver" (it's not the bio owner), so it must use
Darrick> the correct iterator function.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: [PATCH v2] block: fix regression where bio_integrity_process uses wrong bio_vec iterator
  2014-11-26  4:57 ` Martin K. Petersen
@ 2014-12-02 15:15   ` Jens Axboe
  0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2014-12-02 15:15 UTC (permalink / raw)
  To: Martin K. Petersen, Darrick J. Wong; +Cc: linux-kernel, linux-fsdevel

On 11/25/2014 09:57 PM, Martin K. Petersen wrote:
>>>>>> "Darrick" == Darrick J Wong <darrick.wong@oracle.com> writes:
>
> Darrick> Therefore, teach bio_integrity_process() to use
> Darrick> bio_for_each_segment() to iterate the bio_vecs, since the
> Darrick> per-bio iterator tracks which bio_vecs are associated with that
> Darrick> particular bio.  The integrity handling code is effectively
> Darrick> part of the "driver" (it's not the bio owner), so it must use
> Darrick> the correct iterator function.
>
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

Applied for 3.18.

-- 
Jens Axboe

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

end of thread, other threads:[~2014-12-02 15:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-26  1:40 [PATCH v2] block: fix regression where bio_integrity_process uses wrong bio_vec iterator Darrick J. Wong
2014-11-26  4:57 ` Martin K. Petersen
2014-12-02 15:15   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).