FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 10-02-2012, 08:10 PM
Kent Overstreet
 
Default block: Refactor blk_update_request()

On Tue, Oct 02, 2012 at 02:43:59PM -0400, Vivek Goyal wrote:
> On Mon, Sep 24, 2012 at 03:34:44PM -0700, Kent Overstreet wrote:
> > Converts it to use bio_advance(), simplifying it quite a bit in the
> > process.
> >
> > Note that req_bio_endio() now always calls bio_advance() - which means
> > it always loops over the biovec, not just on partial completions. Don't
> > expect it to affect performance, but worth noting.
> >
> > Tested it by forcing partial updates, and dumping before and after on
> > various bio/bvec fields when doing a partial update.
> >
> > Signed-off-by: Kent Overstreet <koverstreet@google.com>
> > CC: Jens Axboe <axboe@kernel.dk>
> > ---
> > block/blk-core.c | 80 +++++++++-----------------------------------------------
> > 1 file changed, 12 insertions(+), 68 deletions(-)
> >
> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index a17869f..a8a1a9e 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -158,20 +158,10 @@ static void req_bio_endio(struct request *rq, struct bio *bio,
> > else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
> > error = -EIO;
> >
> > - if (unlikely(nbytes > bio->bi_size)) {
> > - printk(KERN_ERR "%s: want %u bytes done, %u left
",
> > - __func__, nbytes, bio->bi_size);
> > - nbytes = bio->bi_size;
> > - }
> > -
>
> You are dropping this warning because nobody is calling req_bio_endio()
> with bytes greater than bio size in current code?

Not dropping it, just moved it to bio_advance()

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 10-02-2012, 08:14 PM
Vivek Goyal
 
Default block: Refactor blk_update_request()

On Tue, Oct 02, 2012 at 01:10:14PM -0700, Kent Overstreet wrote:
> On Tue, Oct 02, 2012 at 02:43:59PM -0400, Vivek Goyal wrote:
> > On Mon, Sep 24, 2012 at 03:34:44PM -0700, Kent Overstreet wrote:
> > > Converts it to use bio_advance(), simplifying it quite a bit in the
> > > process.
> > >
> > > Note that req_bio_endio() now always calls bio_advance() - which means
> > > it always loops over the biovec, not just on partial completions. Don't
> > > expect it to affect performance, but worth noting.
> > >
> > > Tested it by forcing partial updates, and dumping before and after on
> > > various bio/bvec fields when doing a partial update.
> > >
> > > Signed-off-by: Kent Overstreet <koverstreet@google.com>
> > > CC: Jens Axboe <axboe@kernel.dk>
> > > ---
> > > block/blk-core.c | 80 +++++++++-----------------------------------------------
> > > 1 file changed, 12 insertions(+), 68 deletions(-)
> > >
> > > diff --git a/block/blk-core.c b/block/blk-core.c
> > > index a17869f..a8a1a9e 100644
> > > --- a/block/blk-core.c
> > > +++ b/block/blk-core.c
> > > @@ -158,20 +158,10 @@ static void req_bio_endio(struct request *rq, struct bio *bio,
> > > else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
> > > error = -EIO;
> > >
> > > - if (unlikely(nbytes > bio->bi_size)) {
> > > - printk(KERN_ERR "%s: want %u bytes done, %u left
",
> > > - __func__, nbytes, bio->bi_size);
> > > - nbytes = bio->bi_size;
> > > - }
> > > -
> >
> > You are dropping this warning because nobody is calling req_bio_endio()
> > with bytes greater than bio size in current code?
>
> Not dropping it, just moved it to bio_advance()

bio_advance() is checking bio vec count and idx and not nr_bytes.

Thanks
Vivek

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 10-02-2012, 08:25 PM
Kent Overstreet
 
Default block: Refactor blk_update_request()

On Tue, Oct 02, 2012 at 04:14:51PM -0400, Vivek Goyal wrote:
> On Tue, Oct 02, 2012 at 01:10:14PM -0700, Kent Overstreet wrote:
> > On Tue, Oct 02, 2012 at 02:43:59PM -0400, Vivek Goyal wrote:
> > > On Mon, Sep 24, 2012 at 03:34:44PM -0700, Kent Overstreet wrote:
> > > > Converts it to use bio_advance(), simplifying it quite a bit in the
> > > > process.
> > > >
> > > > Note that req_bio_endio() now always calls bio_advance() - which means
> > > > it always loops over the biovec, not just on partial completions. Don't
> > > > expect it to affect performance, but worth noting.
> > > >
> > > > Tested it by forcing partial updates, and dumping before and after on
> > > > various bio/bvec fields when doing a partial update.
> > > >
> > > > Signed-off-by: Kent Overstreet <koverstreet@google.com>
> > > > CC: Jens Axboe <axboe@kernel.dk>
> > > > ---
> > > > block/blk-core.c | 80 +++++++++-----------------------------------------------
> > > > 1 file changed, 12 insertions(+), 68 deletions(-)
> > > >
> > > > diff --git a/block/blk-core.c b/block/blk-core.c
> > > > index a17869f..a8a1a9e 100644
> > > > --- a/block/blk-core.c
> > > > +++ b/block/blk-core.c
> > > > @@ -158,20 +158,10 @@ static void req_bio_endio(struct request *rq, struct bio *bio,
> > > > else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
> > > > error = -EIO;
> > > >
> > > > - if (unlikely(nbytes > bio->bi_size)) {
> > > > - printk(KERN_ERR "%s: want %u bytes done, %u left
",
> > > > - __func__, nbytes, bio->bi_size);
> > > > - nbytes = bio->bi_size;
> > > > - }
> > > > -
> > >
> > > You are dropping this warning because nobody is calling req_bio_endio()
> > > with bytes greater than bio size in current code?
> >
> > Not dropping it, just moved it to bio_advance()
>
> bio_advance() is checking bio vec count and idx and not nr_bytes.

Whoops, -ENOCOFFEE... I didn't fully read that code fragment.

Yes, req_bio_endio() is only called from one place, and
blk_update_request() never calls it with nbytes > bio->bi_size (and
after the refactor it's more obviously impossible).

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 10-15-2012, 08:08 PM
Kent Overstreet
 
Default block: Refactor blk_update_request()

Converts it to use bio_advance(), simplifying it quite a bit in the
process.

Note that req_bio_endio() now always calls bio_advance() - which means
it always loops over the biovec, not just on partial completions. Don't
expect it to affect performance, but worth noting.

Tested it by forcing partial updates, and dumping before and after on
various bio/bvec fields when doing a partial update.

Signed-off-by: Kent Overstreet <koverstreet@google.com>
CC: Jens Axboe <axboe@kernel.dk>
---
block/blk-core.c | 80 +++++++++-----------------------------------------------
1 file changed, 12 insertions(+), 68 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index a17869f..a8a1a9e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -158,20 +158,10 @@ static void req_bio_endio(struct request *rq, struct bio *bio,
else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
error = -EIO;

- if (unlikely(nbytes > bio->bi_size)) {
- printk(KERN_ERR "%s: want %u bytes done, %u left
",
- __func__, nbytes, bio->bi_size);
- nbytes = bio->bi_size;
- }
-
if (unlikely(rq->cmd_flags & REQ_QUIET))
set_bit(BIO_QUIET, &bio->bi_flags);

- bio->bi_size -= nbytes;
- bio->bi_sector += (nbytes >> 9);
-
- if (bio_integrity(bio))
- bio_integrity_advance(bio, nbytes);
+ bio_advance(bio, nbytes);

/* don't actually finish bio if it's part of flush sequence */
if (bio->bi_size == 0 && !(rq->cmd_flags & REQ_FLUSH_SEQ))
@@ -2219,8 +2209,7 @@ EXPORT_SYMBOL(blk_fetch_request);
**/
bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
{
- int total_bytes, bio_nbytes, next_idx = 0;
- struct bio *bio;
+ int total_bytes;

if (!req->bio)
return false;
@@ -2264,56 +2253,21 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)

blk_account_io_completion(req, nr_bytes);

- total_bytes = bio_nbytes = 0;
- while ((bio = req->bio) != NULL) {
- int nbytes;
+ total_bytes = 0;
+ while (req->bio) {
+ struct bio *bio = req->bio;
+ unsigned bio_bytes = min(bio->bi_size, nr_bytes);

- if (nr_bytes >= bio->bi_size) {
+ if (bio_bytes == bio->bi_size)
req->bio = bio->bi_next;
- nbytes = bio->bi_size;
- req_bio_endio(req, bio, nbytes, error);
- next_idx = 0;
- bio_nbytes = 0;
- } else {
- int idx = bio->bi_idx + next_idx;
-
- if (unlikely(idx >= bio->bi_vcnt)) {
- blk_dump_rq_flags(req, "__end_that");
- printk(KERN_ERR "%s: bio idx %d >= vcnt %d
",
- __func__, idx, bio->bi_vcnt);
- break;
- }
-
- nbytes = bio_iovec_idx(bio, idx)->bv_len;
- BIO_BUG_ON(nbytes > bio->bi_size);
-
- /*
- * not a complete bvec done
- */
- if (unlikely(nbytes > nr_bytes)) {
- bio_nbytes += nr_bytes;
- total_bytes += nr_bytes;
- break;
- }

- /*
- * advance to the next vector
- */
- next_idx++;
- bio_nbytes += nbytes;
- }
+ req_bio_endio(req, bio, bio_bytes, error);

- total_bytes += nbytes;
- nr_bytes -= nbytes;
+ total_bytes += bio_bytes;
+ nr_bytes -= bio_bytes;

- bio = req->bio;
- if (bio) {
- /*
- * end more in this run, or just return 'not-done'
- */
- if (unlikely(nr_bytes <= 0))
- break;
- }
+ if (!nr_bytes)
+ break;
}

/*
@@ -2329,16 +2283,6 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
return false;
}

- /*
- * if the request wasn't completed, update state
- */
- if (bio_nbytes) {
- req_bio_endio(req, bio, bio_nbytes, error);
- bio->bi_idx += next_idx;
- bio_iovec(bio)->bv_offset += nr_bytes;
- bio_iovec(bio)->bv_len -= nr_bytes;
- }
-
req->__data_len -= total_bytes;
req->buffer = bio_data(req->bio);

--
1.7.12

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 07:15 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org