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

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

2016-06-22 Thread Maxim Patlasov


On 06/22/2016 06:41 AM, Dmitry Monakhov wrote:

Maxim Patlasov  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 
---
  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();
  
@@ -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
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

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

2016-06-22 Thread Dmitry Monakhov

Maxim Patlasov  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 
> ---
>  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();
>  
> @@ -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
  ..
 }
> + 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);
>

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

2016-06-22 Thread Dmitry Monakhov

Maxim Patlasov  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 
> ---
>  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();
>  
> @@ -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 |

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

2016-06-21 Thread Maxim Patlasov

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 
---
 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();
 
@@ -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);
}
 
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;
-

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information