Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1614927 > unrolled thread

[PATCH 2/7] bio-integrity: save original iterator for verify stage

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/7] bio-integrity: save original iterator for verify stage Dmitry Monakhov <dmonakhov@openvz.org> - 2017-04-03 09:30 +0200
    Re: [PATCH 2/7] bio-integrity: save original iterator for verify  stage Christoph Hellwig <hch@infradead.org> - 2017-04-04 09:10 +0200
      Re: [PATCH 2/7] bio-integrity: save original iterator for verify stage Dmitry Monakhov <dmonakhov@openvz.org> - 2017-04-04 14:20 +0200

#1614927 — [PATCH 2/7] bio-integrity: save original iterator for verify stage

In order to perform verification we need to know original data vector
But, after bio traverse io-stack it may be advanced, splited and relocated
many times so it is hard to guess original data vector.

In fact currently ->verify_fn not woks at all because at the moment
it it called bio->bi_iter.bi_size == 0

The simplest way to fix that is to save original data vector and treat is
as immutable.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 block/bio-integrity.c | 6 ++++--
 include/linux/bio.h   | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index b5009a8..43a4476 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -238,10 +238,10 @@ static int bio_integrity_process(struct bio *bio,
 
 	iter.disk_name = bio->bi_bdev->bd_disk->disk_name;
 	iter.interval = 1 << bi->interval_exp;
-	iter.seed = bip_get_seed(bip);
+	iter.seed = bip->bip_verify_iter.bi_sector;
 	iter.prot_buf = prot_buf;
 
-	bio_for_each_segment(bv, bio, bviter) {
+	__bio_for_each_segment(bv, bio, bviter, bip->bip_verify_iter) {
 		void *kaddr = kmap_atomic(bv.bv_page);
 
 		iter.data_buf = kaddr + bv.bv_offset;
@@ -310,6 +310,7 @@ int bio_integrity_prep(struct bio *bio)
 	bip->bip_flags |= BIP_BLOCK_INTEGRITY;
 	bip->bip_iter.bi_size = len;
 	bip_set_seed(bip, bio->bi_iter.bi_sector);
+	bip->bip_verify_iter = bio->bi_iter;
 
 	if (bi->flags & BLK_INTEGRITY_IP_CHECKSUM)
 		bip->bip_flags |= BIP_IP_CHECKSUM;
@@ -476,6 +477,7 @@ int bio_integrity_clone(struct bio *bio, struct bio *bio_src,
 
 	bip->bip_vcnt = bip_src->bip_vcnt;
 	bip->bip_iter = bip_src->bip_iter;
+	bip->bip_verify_iter = bip_src->bip_verify_iter;
 
 	return 0;
 }
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 8e52119..00b086a 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -308,6 +308,7 @@ struct bio_integrity_payload {
 	struct bio		*bip_bio;	/* parent bio */
 
 	struct bvec_iter	bip_iter;
+	struct bvec_iter	bip_verify_iter;/* saved orig data iterator */
 
 	bio_end_io_t		*bip_end_io;	/* saved I/O completion fn */
 
-- 
2.9.3

[toc] | [next] | [standalone]

#1615737 — Re: [PATCH 2/7] bio-integrity: save original iterator for verify stage

This is a pretty big increase in the bio_integrity_payload size,
but I guess we can't get around it..

Reviewed-by: Christoph Hellwig <hch@lst.de>

[toc] | [prev] | [next] | [standalone]

#1615946

Christoph Hellwig <hch@infradead.org> writes:

> This is a pretty big increase in the bio_integrity_payload size,
> but I guess we can't get around it..

Yes, everybody hate this solution, me too, but I've stated with
other approach and it is appeaded to be very ugly.

My idea was that we have two types of iterator incrementors: bio_advance()
and bio_xx_complete, First one is called during split, later is called
on completion ( req_bio_endio() ) . So we can add new field "bi_done" to
iterator which has similar meaning as bi_bvec_done, but at full iterator
scope.  It is incremented during completion, but before end_io.
Chain bios will propogate bi_done to parent bio to parent one.
On ->vefify_fn() iterator will be rewinded (counter part of bvec_advance) to
iter->bi_done bytes, so we will get oritinal iterator. 
I've even prepare a patch for this idea and it looks big and awful.
Even more it does not works if chained bios overlapts (raid1,raid10,
etc).

But... at the time I've wrote this email I've realized that I do not
care about what happen with chained bios. The only thing is important
is parent bio and how far it was advanced. If bi_done is incremented
inside bvec_iter_advance() I can be shure that at the moment
->bi_end_io()
original position can be restored by rewinding back to io_done bytes.

I'll try to implement this.

>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web