[Devel] [PATCH rh7] ploop: fix barriers for ordinary requests

[Dmitry Monakhov]

Maxim Patlasov <mpatlasov at virtuozzo.com> writes:

> The way how io_direct.c handles FLUSH|FUA: b1:FLUSH,b2,b3,b4,b5:FLUSH|FUA
> is completely wrong: to make sure that b1:FLUSH made effect we have to
> wait for its completion. Similarly, even if we're sure that FUA will be
> processed as post-FLUSH (also dubious!), we have to wait for completion
> b1..b4 to make sure that that flush will cover them.
>
> The patch fixes all these issues pretty simple: let's mark outgouing
> bio-s with FLUSH|FUA based on those flags in *corresponing* incoming
> bio-s.
>
> Signed-off-by: Maxim Patlasov <mpatlasov at virtuozzo.com>
> ---
>  drivers/block/ploop/dev.c       |    1 -
>  drivers/block/ploop/io_direct.c |   47 ++++++++++++---------------------------
>  2 files changed, 15 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
> index 2ef1449..6b5702f 100644
> --- a/drivers/block/ploop/dev.c
> +++ b/drivers/block/ploop/dev.c
> @@ -498,7 +498,6 @@ ploop_bio_queue(struct ploop_device * plo, struct bio * bio,
>  	preq->req_sector = bio->bi_sector;
>  	preq->req_size = bio->bi_size >> 9;
>  	preq->req_rw = bio->bi_rw;
> -	bio->bi_rw &= ~(REQ_FLUSH | REQ_FUA);
Wow. I can't even imagine that we clear barrier flags from original bios
>  	preq->eng_state = PLOOP_E_ENTRY;
>  	preq->state = 0;
>  	preq->error = 0;
> diff --git a/drivers/block/ploop/io_direct.c b/drivers/block/ploop/io_direct.c
> index 6ef9cd8..84c9a48 100644
> --- a/drivers/block/ploop/io_direct.c
> +++ b/drivers/block/ploop/io_direct.c
> @@ -92,7 +92,6 @@ dio_submit(struct ploop_io *io, struct ploop_request * preq,
>  	int preflush;
>  	int postfua = 0;
>  	int write = !!(rw & REQ_WRITE);
> -	int bio_num;
>  
>  	trace_submit(preq);
>  
> @@ -233,13 +232,13 @@ flush_bio:
>  			goto flush_bio;
>  		}
>  
> +		bio->bi_rw |= bw.cur->bi_rw & (REQ_FLUSH | REQ_FUA);
>  		bw.bv_off += copy;
>  		size -= copy >> 9;
>  		sec += copy >> 9;
>  	}
>  	ploop_extent_put(em);
>  
> -	bio_num = 0;
>  	while (bl.head) {
>  		struct bio * b = bl.head;
>  		unsigned long rw2 = rw;
> @@ -255,11 +254,10 @@ flush_bio:
>  			preflush = 0;
>  		}
>  		if (unlikely(postfua && !bl.head))
> -			rw2 |= (REQ_FUA | ((bio_num) ? REQ_FLUSH : 0));
> +			rw2 |= REQ_FUA;
>  
>  		ploop_acc_ff_out(preq->plo, rw2 | b->bi_rw);
> -		submit_bio(rw2, b);
> -		bio_num++;
> +		submit_bio(rw2 | b->bi_rw, b);
>  	}
>  
>  	ploop_complete_io_request(preq);
> @@ -567,7 +565,6 @@ dio_submit_pad(struct ploop_io *io, struct ploop_request * preq,
>  	sector_t sec, end_sec, nsec, start, end;
>  	struct bio_list_walk bw;
>  	int err;
> -	int preflush = !!(preq->req_rw & REQ_FLUSH);
>  
>  	bio_list_init(&bl);
>  
> @@ -598,14 +595,17 @@ dio_submit_pad(struct ploop_io *io, struct ploop_request * preq,
>  	while (sec < end_sec) {
>  		struct page * page;
>  		unsigned int poff, plen;
> +		bool zero_page;
>  
>  		if (sec < start) {
> +			zero_page = true;
>  			page = ZERO_PAGE(0);
>  			poff = 0;
>  			plen = start - sec;
>  			if (plen > (PAGE_SIZE>>9))
>  				plen = (PAGE_SIZE>>9);
>  		} else if (sec >= end) {
> +			zero_page = true;
>  			page = ZERO_PAGE(0);
>  			poff = 0;
>  			plen = end_sec - sec;
> @@ -614,6 +614,7 @@ dio_submit_pad(struct ploop_io *io, struct ploop_request * preq,
>  		} else {
>  			/* sec >= start && sec < end */
>  			struct bio_vec * bv;
> +			zero_page = false;
>  
>  			if (sec == start) {
>  				bw.cur = sbl->head;
> @@ -672,6 +673,10 @@ flush_bio:
>  			goto flush_bio;
>  		}
>  
> +		/* Handle FLUSH here, dio_post_submit will handle FUA */
> +		if (!zero_page)
> +			bio->bi_rw |= bw.cur->bi_rw & REQ_FLUSH;
> +
>  		bw.bv_off += (plen<<9);
>  		BUG_ON(plen == 0);
>  		sec += plen;
> @@ -688,13 +693,9 @@ flush_bio:
>  		b->bi_private = preq;
>  		b->bi_end_io = dio_endio_async;
>  
> -		rw = sbl->head->bi_rw | WRITE;
> -		if (unlikely(preflush)) {
> -			rw |= REQ_FLUSH;
> -			preflush = 0;
> -		}
> +		rw = preq->req_rw & ~(REQ_FLUSH | REQ_FUA);
>  		ploop_acc_ff_out(preq->plo, rw | b->bi_rw);
> -		submit_bio(rw, b);
> +		submit_bio(rw | b->bi_rw, b);
This is useless statement ^^^^^^^^^^^^^^
submit_bio looks like follows:
void submit_bio(int rw, struct bio *bio)
{
        bio->bi_rw |= rw;
.....        
>  	}
>  
>  	ploop_complete_io_request(preq);
> @@ -1422,13 +1423,6 @@ dio_io_page(struct ploop_io * io, unsigned long rw,
>  	sector_t nsec;
>  	int err;
>  	int off;
> -	int postfua;
> -	int bio_num;
> -	int preflush;
> -
> -	preflush = !!(rw & REQ_FLUSH);
> -	postfua = !!(rw & REQ_FUA);
> -	rw &= ~(REQ_FUA|REQ_FLUSH);
>  
>  	bio_list_init(&bl);
>  	bio = NULL;
> @@ -1481,27 +1475,16 @@ flush_bio:
>  	if (em)
>  		ploop_extent_put(em);
>  
> -	bio_num = 0;
>  	while (bl.head) {
> -		unsigned long rw2 = rw;
>  		struct bio * b = bl.head;
>  		bl.head = b->bi_next;
>  
> -		if (unlikely(preflush)) {
> -			rw2 |= REQ_FLUSH;
> -			preflush = 0;
> -		}
> -
> -		if (unlikely(postfua && !bl.head))
> -			rw2 |= (REQ_FUA | ((bio_num) ? REQ_FLUSH : 0));
> -
>  		b->bi_next = NULL;
>  		b->bi_end_io = dio_endio_async;
>  		b->bi_private = preq;
>  		atomic_inc(&preq->io_count);
> -		ploop_acc_ff_out(preq->plo, rw2 | b->bi_rw);
> -		submit_bio(rw2, b);
> -		bio_num++;
> +		ploop_acc_ff_out(preq->plo, rw | b->bi_rw);
> +		submit_bio(rw, b);
>  	}
>  
>  	ploop_complete_io_request(preq);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 472 bytes
Desc: not available
URL: <http://lists.openvz.org/pipermail/devel/attachments/20160622/69c981a4/attachment.sig>