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

[Maxim Patlasov]

On 06/22/2016 06:41 AM, Dmitry Monakhov wrote:
> 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.
> One more thing please see below.
>> 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);
>>   	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 */
> submit_pad may be called w/o post_submit flag from here:
> ->dio_submit_alloc
>    if (io->files.em_tree->_get_extent) {
>     ->dio_fallocate
>     ->dio_submit_pad
>    ..
>   }

We never has _get_extent set. This is legacy code for PCSS support, 
we'll remove it. For now, we can safely ignore this.

Thanks,
Maxim