| Message ID | 20220425075418.2192130-11-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] btrfs: move more work into btrfs_end_bioc | expand |
On 2022/4/25 15:54, Christoph Hellwig wrote: > The bios submitted from btrfs_map_bio don't really interact with the > rest of btrfs and the only btrfs_bio member actually used in the > low-level bios is the pointer to the btrfs_io_contex used for endio > handler. > > Use a union in struct btrfs_io_stripe that allows the endio handler to > find the btrfs_io_context and remove the spurious ->device assignment > so that a plain fs_bio_set bio can be used for the low-level bios > allocated inside btrfs_map_bio. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/btrfs/extent_io.c | 13 ------------- > fs/btrfs/extent_io.h | 1 - > fs/btrfs/volumes.c | 20 ++++++++++---------- > fs/btrfs/volumes.h | 5 ++++- > 4 files changed, 14 insertions(+), 25 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index a14ed9b9dc2d0..37f4eee418219 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -3209,19 +3209,6 @@ struct bio *btrfs_bio_alloc(unsigned int nr_iovecs) > return bio; > } > > -struct bio *btrfs_bio_clone(struct block_device *bdev, struct bio *bio) > -{ > - struct btrfs_bio *bbio; > - struct bio *new; > - > - /* Bio allocation backed by a bioset does not fail */ > - new = bio_alloc_clone(bdev, bio, GFP_NOFS, &btrfs_bioset); > - bbio = btrfs_bio(new); > - btrfs_bio_init(bbio); > - bbio->iter = bio->bi_iter; > - return new; > -} > - > struct bio *btrfs_bio_clone_partial(struct bio *orig, u64 offset, u64 size) > { > struct bio *bio; > diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h > index b390ec79f9a86..3078e90be3a99 100644 > --- a/fs/btrfs/extent_io.h > +++ b/fs/btrfs/extent_io.h > @@ -265,7 +265,6 @@ void extent_clear_unlock_delalloc(struct btrfs_inode *inode, u64 start, u64 end, > > int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array); > struct bio *btrfs_bio_alloc(unsigned int nr_iovecs); > -struct bio *btrfs_bio_clone(struct block_device *bdev, struct bio *bio); > struct bio *btrfs_bio_clone_partial(struct bio *orig, u64 offset, u64 size); > > void end_extent_writepage(struct page *page, int err, u64 start, u64 end); > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index d54aacb4f05f2..c621bd631450a 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -6666,23 +6666,21 @@ static void btrfs_end_bioc(struct btrfs_io_context *bioc, bool async) > > static void btrfs_end_bio(struct bio *bio) > { > - struct btrfs_io_context *bioc = bio->bi_private; > + struct btrfs_io_stripe *stripe = bio->bi_private; > + struct btrfs_io_context *bioc = stripe->bioc; > > if (bio->bi_status) { > atomic_inc(&bioc->error); > if (bio->bi_status == BLK_STS_IOERR || > bio->bi_status == BLK_STS_TARGET) { > - struct btrfs_device *dev = btrfs_bio(bio)->device; > - > - ASSERT(dev->bdev); > if (btrfs_op(bio) == BTRFS_MAP_WRITE) > - btrfs_dev_stat_inc_and_print(dev, > + btrfs_dev_stat_inc_and_print(stripe->dev, > BTRFS_DEV_STAT_WRITE_ERRS); > else if (!(bio->bi_opf & REQ_RAHEAD)) > - btrfs_dev_stat_inc_and_print(dev, > + btrfs_dev_stat_inc_and_print(stripe->dev, > BTRFS_DEV_STAT_READ_ERRS); > if (bio->bi_opf & REQ_PREFLUSH) > - btrfs_dev_stat_inc_and_print(dev, > + btrfs_dev_stat_inc_and_print(stripe->dev, > BTRFS_DEV_STAT_FLUSH_ERRS); > } > } > @@ -6714,14 +6712,16 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, > } > > if (clone) { > - bio = btrfs_bio_clone(dev->bdev, orig_bio); > + bio = bio_alloc_clone(dev->bdev, orig_bio, GFP_NOFS, > + &fs_bio_set); > } else { > bio = orig_bio; > bio_set_dev(bio, dev->bdev); > + btrfs_bio(bio)->device = dev; > } > > - bio->bi_private = bioc; > - btrfs_bio(bio)->device = dev; > + bioc->stripes[dev_nr].bioc = bioc; > + bio->bi_private = &bioc->stripes[dev_nr]; > bio->bi_end_io = btrfs_end_bio; > bio->bi_iter.bi_sector = physical >> 9; > /* > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 28e28b7c48649..825e44c82f2b0 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -396,7 +396,10 @@ static inline void btrfs_bio_free_csum(struct btrfs_bio *bbio) > > struct btrfs_io_stripe { > struct btrfs_device *dev; > - u64 physical; > + union { > + u64 physical; /* block mapping */ > + struct btrfs_io_context *bioc; /* for the endio handler */ > + }; > u64 length; /* only used for discard mappings */ Isn't @length a better candidate? Since it's only used for discard. Thanks, Qu > }; >
On Mon, Apr 25, 2022 at 05:01:11PM +0800, Qu Wenruo wrote: >> struct btrfs_io_stripe { >> struct btrfs_device *dev; >> - u64 physical; >> + union { >> + u64 physical; /* block mapping */ >> + struct btrfs_io_context *bioc; /* for the endio handler */ >> + }; >> u64 length; /* only used for discard mappings */ > > Isn't @length a better candidate? > > Since it's only used for discard. I have anoter patch to be sumitted that removes length entirely by not using btrfs_io_stripe for discards.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index a14ed9b9dc2d0..37f4eee418219 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3209,19 +3209,6 @@ struct bio *btrfs_bio_alloc(unsigned int nr_iovecs) return bio; } -struct bio *btrfs_bio_clone(struct block_device *bdev, struct bio *bio) -{ - struct btrfs_bio *bbio; - struct bio *new; - - /* Bio allocation backed by a bioset does not fail */ - new = bio_alloc_clone(bdev, bio, GFP_NOFS, &btrfs_bioset); - bbio = btrfs_bio(new); - btrfs_bio_init(bbio); - bbio->iter = bio->bi_iter; - return new; -} - struct bio *btrfs_bio_clone_partial(struct bio *orig, u64 offset, u64 size) { struct bio *bio; diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index b390ec79f9a86..3078e90be3a99 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -265,7 +265,6 @@ void extent_clear_unlock_delalloc(struct btrfs_inode *inode, u64 start, u64 end, int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array); struct bio *btrfs_bio_alloc(unsigned int nr_iovecs); -struct bio *btrfs_bio_clone(struct block_device *bdev, struct bio *bio); struct bio *btrfs_bio_clone_partial(struct bio *orig, u64 offset, u64 size); void end_extent_writepage(struct page *page, int err, u64 start, u64 end); diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index d54aacb4f05f2..c621bd631450a 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6666,23 +6666,21 @@ static void btrfs_end_bioc(struct btrfs_io_context *bioc, bool async) static void btrfs_end_bio(struct bio *bio) { - struct btrfs_io_context *bioc = bio->bi_private; + struct btrfs_io_stripe *stripe = bio->bi_private; + struct btrfs_io_context *bioc = stripe->bioc; if (bio->bi_status) { atomic_inc(&bioc->error); if (bio->bi_status == BLK_STS_IOERR || bio->bi_status == BLK_STS_TARGET) { - struct btrfs_device *dev = btrfs_bio(bio)->device; - - ASSERT(dev->bdev); if (btrfs_op(bio) == BTRFS_MAP_WRITE) - btrfs_dev_stat_inc_and_print(dev, + btrfs_dev_stat_inc_and_print(stripe->dev, BTRFS_DEV_STAT_WRITE_ERRS); else if (!(bio->bi_opf & REQ_RAHEAD)) - btrfs_dev_stat_inc_and_print(dev, + btrfs_dev_stat_inc_and_print(stripe->dev, BTRFS_DEV_STAT_READ_ERRS); if (bio->bi_opf & REQ_PREFLUSH) - btrfs_dev_stat_inc_and_print(dev, + btrfs_dev_stat_inc_and_print(stripe->dev, BTRFS_DEV_STAT_FLUSH_ERRS); } } @@ -6714,14 +6712,16 @@ static void submit_stripe_bio(struct btrfs_io_context *bioc, } if (clone) { - bio = btrfs_bio_clone(dev->bdev, orig_bio); + bio = bio_alloc_clone(dev->bdev, orig_bio, GFP_NOFS, + &fs_bio_set); } else { bio = orig_bio; bio_set_dev(bio, dev->bdev); + btrfs_bio(bio)->device = dev; } - bio->bi_private = bioc; - btrfs_bio(bio)->device = dev; + bioc->stripes[dev_nr].bioc = bioc; + bio->bi_private = &bioc->stripes[dev_nr]; bio->bi_end_io = btrfs_end_bio; bio->bi_iter.bi_sector = physical >> 9; /* diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 28e28b7c48649..825e44c82f2b0 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -396,7 +396,10 @@ static inline void btrfs_bio_free_csum(struct btrfs_bio *bbio) struct btrfs_io_stripe { struct btrfs_device *dev; - u64 physical; + union { + u64 physical; /* block mapping */ + struct btrfs_io_context *bioc; /* for the endio handler */ + }; u64 length; /* only used for discard mappings */ };
The bios submitted from btrfs_map_bio don't really interact with the rest of btrfs and the only btrfs_bio member actually used in the low-level bios is the pointer to the btrfs_io_contex used for endio handler. Use a union in struct btrfs_io_stripe that allows the endio handler to find the btrfs_io_context and remove the spurious ->device assignment so that a plain fs_bio_set bio can be used for the low-level bios allocated inside btrfs_map_bio. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/extent_io.c | 13 ------------- fs/btrfs/extent_io.h | 1 - fs/btrfs/volumes.c | 20 ++++++++++---------- fs/btrfs/volumes.h | 5 ++++- 4 files changed, 14 insertions(+), 25 deletions(-)