| Message ID | 20201206055332.3144-2-tom.ty89@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/3] block: try one write zeroes request before going further | expand |
On 12/6/20 6:53 AM, Tom Yan wrote: > Instead of using the same check for the two layers of loops, count > bio pages in the inner loop instead. > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> > --- > block/blk-lib.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/block/blk-lib.c b/block/blk-lib.c > index c1e9388a8fb8..354dcab760c7 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -318,7 +318,7 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev, > struct request_queue *q = bdev_get_queue(bdev); > struct bio *bio = *biop; > int bi_size = 0; > - unsigned int sz; > + unsigned int sz, bio_nr_pages; > > if (!q) > return -ENXIO; > @@ -327,19 +327,18 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev, > return -EPERM; > > while (nr_sects != 0) { > - bio = blk_next_bio(bio, __blkdev_sectors_to_bio_pages(nr_sects), > - gfp_mask); > + bio_nr_pages = __blkdev_sectors_to_bio_pages(nr_sects); > + bio = blk_next_bio(bio, bio_nr_pages, gfp_mask); > bio->bi_iter.bi_sector = sector; > bio_set_dev(bio, bdev); > bio_set_op_attrs(bio, REQ_OP_WRITE, 0); > > - while (nr_sects != 0) { > + while (bio_nr_pages != 0) { > sz = min((sector_t) PAGE_SIZE, nr_sects << 9); nr_sects will need to be modified, too, if we iterate over bio_nr_pages instead of nr_sects. > bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0); > nr_sects -= bi_size >> 9; > sector += bi_size >> 9; > - if (bi_size < sz) > - break; > + bio_nr_pages--; > } > cond_resched(); > } > Cheers, Hannes
On Sun, 6 Dec 2020 at 19:29, Hannes Reinecke <hare@suse.de> wrote: > > On 12/6/20 6:53 AM, Tom Yan wrote: > > Instead of using the same check for the two layers of loops, count > > bio pages in the inner loop instead. > > > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> > > --- > > block/blk-lib.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/block/blk-lib.c b/block/blk-lib.c > > index c1e9388a8fb8..354dcab760c7 100644 > > --- a/block/blk-lib.c > > +++ b/block/blk-lib.c > > @@ -318,7 +318,7 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev, > > struct request_queue *q = bdev_get_queue(bdev); > > struct bio *bio = *biop; > > int bi_size = 0; > > - unsigned int sz; > > + unsigned int sz, bio_nr_pages; > > > > if (!q) > > return -ENXIO; > > @@ -327,19 +327,18 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev, > > return -EPERM; > > > > while (nr_sects != 0) { > > - bio = blk_next_bio(bio, __blkdev_sectors_to_bio_pages(nr_sects), > > - gfp_mask); > > + bio_nr_pages = __blkdev_sectors_to_bio_pages(nr_sects); > > + bio = blk_next_bio(bio, bio_nr_pages, gfp_mask); > > bio->bi_iter.bi_sector = sector; > > bio_set_dev(bio, bdev); > > bio_set_op_attrs(bio, REQ_OP_WRITE, 0); > > > > - while (nr_sects != 0) { > > + while (bio_nr_pages != 0) { > > sz = min((sector_t) PAGE_SIZE, nr_sects << 9); > > nr_sects will need to be modified, too, if we iterate over bio_nr_pages > instead of nr_sects. > > > bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0); > > nr_sects -= bi_size >> 9; Not sure what modification you are suggesting. We are still deducting from it the "added" sectors. > > sector += bi_size >> 9; > > - if (bi_size < sz) > > - break; > > + bio_nr_pages--; > > } > > cond_resched(); > > } > > > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Kernel Storage Architect > hare@suse.de +49 911 74053 688 > SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg > HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On Sun, Dec 06, 2020 at 01:53:31PM +0800, Tom Yan wrote: > Instead of using the same check for the two layers of loops, count > bio pages in the inner loop instead. I don't really see any major benefit of one version over the other.
Sure. It's only for code clarity, so definitely no "major benefit". On Mon, 7 Dec 2020 at 21:34, Christoph Hellwig <hch@infradead.org> wrote: > > On Sun, Dec 06, 2020 at 01:53:31PM +0800, Tom Yan wrote: > > Instead of using the same check for the two layers of loops, count > > bio pages in the inner loop instead. > > I don't really see any major benefit of one version over the other.
diff --git a/block/blk-lib.c b/block/blk-lib.c index c1e9388a8fb8..354dcab760c7 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -318,7 +318,7 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev, struct request_queue *q = bdev_get_queue(bdev); struct bio *bio = *biop; int bi_size = 0; - unsigned int sz; + unsigned int sz, bio_nr_pages; if (!q) return -ENXIO; @@ -327,19 +327,18 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev, return -EPERM; while (nr_sects != 0) { - bio = blk_next_bio(bio, __blkdev_sectors_to_bio_pages(nr_sects), - gfp_mask); + bio_nr_pages = __blkdev_sectors_to_bio_pages(nr_sects); + bio = blk_next_bio(bio, bio_nr_pages, gfp_mask); bio->bi_iter.bi_sector = sector; bio_set_dev(bio, bdev); bio_set_op_attrs(bio, REQ_OP_WRITE, 0); - while (nr_sects != 0) { + while (bio_nr_pages != 0) { sz = min((sector_t) PAGE_SIZE, nr_sects << 9); bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0); nr_sects -= bi_size >> 9; sector += bi_size >> 9; - if (bi_size < sz) - break; + bio_nr_pages--; } cond_resched(); }
Instead of using the same check for the two layers of loops, count bio pages in the inner loop instead. Signed-off-by: Tom Yan <tom.ty89@gmail.com> --- block/blk-lib.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)