| Message ID | 1482854250-13481-24-git-send-email-tom.leiming@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2016/12/27 下午11:56, Ming Lei wrote: > The incoming bio may be too big to be cloned into > one singlepage bvecs bio, so split the bio and > check the splitted bio one by one. > > Signed-off-by: Ming Lei <tom.leiming@gmail.com> > --- > drivers/md/bcache/debug.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c > index 48d03e8b3385..18b2d2d138e3 100644 > --- a/drivers/md/bcache/debug.c > +++ b/drivers/md/bcache/debug.c > @@ -103,7 +103,7 @@ void bch_btree_verify(struct btree *b) > up(&b->io_mutex); > } > > -void bch_data_verify(struct cached_dev *dc, struct bio *bio) > +static void __bch_data_verify(struct cached_dev *dc, struct bio *bio) > { > char name[BDEVNAME_SIZE]; > struct bio *check; > @@ -116,7 +116,7 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) > * in the new cloned bio because each single page need > * to assign to each bvec of the new bio. > */ > - check = bio_clone(bio, GFP_NOIO); > + check = bio_clone_sp(bio, GFP_NOIO); > if (!check) > return; > check->bi_opf = REQ_OP_READ; > @@ -151,6 +151,26 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) > bio_put(check); > } > > +void bch_data_verify(struct cached_dev *dc, struct bio *bio) > +{ > + struct request_queue *q = bdev_get_queue(bio->bi_bdev); > + struct bio *clone = bio_clone_fast(bio, GFP_NOIO, q->bio_split); > + unsigned sectors; > + > + while (!bio_can_convert_to_sp(clone, §ors)) { > + struct bio *split = bio_split(clone, sectors, > + GFP_NOIO, q->bio_split); > + > + __bch_data_verify(dc, split); > + bio_put(split); > + } > + > + if (bio_sectors(clone)) > + __bch_data_verify(dc, clone); > + > + bio_put(clone); > +} > + Hi Lei, The above patch is good IMHO. Just wondering why not use the classical style ? something like, do { if (!bio_can_convert_to_sp(clone, §ors)) split = bio_split(clone, sectors, GFP_NOIO, q->bio_split); else split = clone; __bch_data_verity(gc, split); bio_put(split); } while (split != clone); I guess maybe the above style generates less binary code.
Hi Coly, On Fri, Dec 30, 2016 at 7:01 PM, Coly Li <i@coly.li> wrote: > On 2016/12/27 下午11:56, Ming Lei wrote: >> The incoming bio may be too big to be cloned into >> one singlepage bvecs bio, so split the bio and >> check the splitted bio one by one. >> >> Signed-off-by: Ming Lei <tom.leiming@gmail.com> >> --- >> drivers/md/bcache/debug.c | 24 ++++++++++++++++++++++-- >> 1 file changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c >> index 48d03e8b3385..18b2d2d138e3 100644 >> --- a/drivers/md/bcache/debug.c >> +++ b/drivers/md/bcache/debug.c >> @@ -103,7 +103,7 @@ void bch_btree_verify(struct btree *b) >> up(&b->io_mutex); >> } >> >> -void bch_data_verify(struct cached_dev *dc, struct bio *bio) >> +static void __bch_data_verify(struct cached_dev *dc, struct bio *bio) >> { >> char name[BDEVNAME_SIZE]; >> struct bio *check; >> @@ -116,7 +116,7 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) >> * in the new cloned bio because each single page need >> * to assign to each bvec of the new bio. >> */ >> - check = bio_clone(bio, GFP_NOIO); >> + check = bio_clone_sp(bio, GFP_NOIO); >> if (!check) >> return; >> check->bi_opf = REQ_OP_READ; >> @@ -151,6 +151,26 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) >> bio_put(check); >> } >> >> +void bch_data_verify(struct cached_dev *dc, struct bio *bio) >> +{ >> + struct request_queue *q = bdev_get_queue(bio->bi_bdev); >> + struct bio *clone = bio_clone_fast(bio, GFP_NOIO, q->bio_split); >> + unsigned sectors; >> + >> + while (!bio_can_convert_to_sp(clone, §ors)) { >> + struct bio *split = bio_split(clone, sectors, >> + GFP_NOIO, q->bio_split); >> + >> + __bch_data_verify(dc, split); >> + bio_put(split); >> + } >> + >> + if (bio_sectors(clone)) >> + __bch_data_verify(dc, clone); >> + >> + bio_put(clone); >> +} >> + > > Hi Lei, > > The above patch is good IMHO. Just wondering why not use the classical > style ? something like, I don't know there is the classical style, :-) > > > do { > if (!bio_can_convert_to_sp(clone, §ors)) > split = bio_split(clone, sectors, > GFP_NOIO, q->bio_split); > else > split = clone; > > __bch_data_verity(gc, split); > bio_put(split); > } while (split != clone); > > > I guess maybe the above style generates less binary code. Maybe, will take this style in V2. Thanks for the review!
diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c index 48d03e8b3385..18b2d2d138e3 100644 --- a/drivers/md/bcache/debug.c +++ b/drivers/md/bcache/debug.c @@ -103,7 +103,7 @@ void bch_btree_verify(struct btree *b) up(&b->io_mutex); } -void bch_data_verify(struct cached_dev *dc, struct bio *bio) +static void __bch_data_verify(struct cached_dev *dc, struct bio *bio) { char name[BDEVNAME_SIZE]; struct bio *check; @@ -116,7 +116,7 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) * in the new cloned bio because each single page need * to assign to each bvec of the new bio. */ - check = bio_clone(bio, GFP_NOIO); + check = bio_clone_sp(bio, GFP_NOIO); if (!check) return; check->bi_opf = REQ_OP_READ; @@ -151,6 +151,26 @@ void bch_data_verify(struct cached_dev *dc, struct bio *bio) bio_put(check); } +void bch_data_verify(struct cached_dev *dc, struct bio *bio) +{ + struct request_queue *q = bdev_get_queue(bio->bi_bdev); + struct bio *clone = bio_clone_fast(bio, GFP_NOIO, q->bio_split); + unsigned sectors; + + while (!bio_can_convert_to_sp(clone, §ors)) { + struct bio *split = bio_split(clone, sectors, + GFP_NOIO, q->bio_split); + + __bch_data_verify(dc, split); + bio_put(split); + } + + if (bio_sectors(clone)) + __bch_data_verify(dc, clone); + + bio_put(clone); +} + #endif #ifdef CONFIG_DEBUG_FS
The incoming bio may be too big to be cloned into one singlepage bvecs bio, so split the bio and check the splitted bio one by one. Signed-off-by: Ming Lei <tom.leiming@gmail.com> --- drivers/md/bcache/debug.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)