More bio fixes and cleanups
David Sterba
maintainercover More bio fixes and cleanups
Main part is the initialization of btrfs_io_bio, the rest is doc & cleanup.
David Sterba (3):
btrfs: document mandatory order of bio in btrfs_io_bio
btrfs: add helper to initialize the non-bio part of btrfs_io_bio
btrfs: sink gfp parameter to btrfs_io_bio_alloc
fs/btrfs/check-integrity.c | 2 +- fs/btrfs/disk-io.c | 2 +- fs/btrfs/extent_io.c | 38 ++++++++++++++++++-------------------- fs/btrfs/extent_io.h | 2 +- fs/btrfs/raid56.c | 2 +- fs/btrfs/scrub.c | 16 +++++++--------- fs/btrfs/volumes.h | 4 ++++ 7 files changed, 33 insertions(+), 33 deletions(-)
Signature · 1 line
2.13.0
David Sterba
maintainerPATCH 2/3 btrfs: add helper to initialize the non-bio part of btrfs_io_bio
We use btrfs_bioset for bios and ask to allocate the entire size of btrfs_io_bio from btrfs bio_alloc_bioset. The member 'bio' is initialized but the bytes from 0 to offset of 'bio' are left uninitialized. Although we initialize some of the members in our helpers, we should initialize the whole structures.
--- fs/btrfs/extent_io.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
fs/btrfs/extent_io.c +14 -16
@@ -2654,22 +2654,28 @@ static void end_bio_extent_readpage(struct bio *bio) }/* +* Initialize the members up to but not including 'bio'. Use after allocating a +* new bio by bio_alloc_bioset as it does not initialize the bytes outside of +* 'bio' because use of __GFP_ZERO is not supported. +*/ +static inline void btrfs_io_bio_init (struct btrfs_io_bio *btrfs_bio) +{ +memset (btrfs_bio,0 , offsetof(struct btrfs_io_bio , bio)); +} + +/* * The following helpers allocate a bio. As it's backed by a bioset, it'll * never fail. We're returning a bio right now but you can call btrfs_io_bio * for the appropriate container_of magic */ struct bio *btrfs_bio_alloc (struct block_device *bdev,u64 first_byte) { -struct btrfs_io_bio *btrfs_bio;struct bio *bio; bio =bio_alloc_bioset (GFP_NOFS ,BIO_MAX_PAGES , btrfs_bioset); bio->bi_bdev = bdev; bio->bi_iter.bi_sector = first_byte >>9 ; - btrfs_bio =btrfs_io_bio (bio); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_io_bio (bio));return bio; }
@@ -2681,24 +2687,18 @@ struct bio *btrfs_bio_clone(struct bio *bio)/* Bio allocation backed by a bioset does not fail */ new =bio_clone_fast (bio,GFP_NOFS , btrfs_bioset); btrfs_bio =btrfs_io_bio (new); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_bio); btrfs_bio->iter = bio->bi_iter;return new; }struct bio *btrfs_io_bio_alloc (gfp_t gfp_mask,unsigned int nr_iovecs) { -struct btrfs_io_bio *btrfs_bio;struct bio *bio;/* Bio allocation backed by a bioset does not fail */ bio =bio_alloc_bioset (gfp_mask, nr_iovecs, btrfs_bioset); - btrfs_bio =btrfs_io_bio (bio); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_io_bio (bio));return bio; }
@@ -2712,9 +2712,7 @@ struct bio *btrfs_bio_clone_partial(struct bio *orig, int offset, int size)ASSERT (bio); btrfs_bio =btrfs_io_bio (bio); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_bio);bio_trim (bio, offset >>9 , size >>9 ); btrfs_bio->iter = bio->bi_iter;
Signature · 1 line
2.13.0
David Sterba
maintainerPATCH 1/3 btrfs: document mandatory order of bio in btrfs_io_bio
In reply to David Sterba--- fs/btrfs/volumes.h | 4 ++++ 1 file changed, 4 insertions(+)
fs/btrfs/volumes.h +4 -0
@@ -281,6 +281,10 @@ struct btrfs_io_bio {u8 *csum_allocated;btrfs_io_bio_end_io_t *end_io;struct bvec_iter iter; +/* +* This member must come last, bio_alloc_bioset will allocate enough +* bytes for entire btrfs_io_bio but relies on bio being last. +*/ struct bio bio; };
Signature · 1 line
2.13.0
David Sterba
maintainerPATCH 3/3 btrfs: sink gfp parameter to btrfs_io_bio_alloc
In reply to David SterbaWe can hardcode GFP_NOFS to btrfs_io_bio_alloc, although it means we change it back from GFP_KERNEL in scrub. I'd rather save a few stack bytes from not passing the gfp flags in the remaining, more imporatant, contexts and the bio allocating API now looks more consistent.
--- fs/btrfs/check-integrity.c | 2 +- fs/btrfs/disk-io.c | 2 +- fs/btrfs/extent_io.c | 8 ++++---- fs/btrfs/extent_io.h | 2 +- fs/btrfs/raid56.c | 2 +- fs/btrfs/scrub.c | 16 +++++++--------- 6 files changed, 15 insertions(+), 17 deletions(-)
fs/btrfs/check-integrity.c +1 -1
@@ -1638,7 +1638,7 @@ static int btrfsic_read_block(struct btrfsic_state *state,struct bio *bio;unsigned int j; - bio =btrfs_io_bio_alloc (GFP_NOFS , num_pages - i); + bio =btrfs_io_bio_alloc (num_pages - i); bio->bi_bdev = block_ctx->dev->bdev; bio->bi_iter.bi_sector = dev_bytenr >>9 ;bio_set_op_attrs (bio,REQ_OP_READ ,0 );
fs/btrfs/disk-io.c +1 -1
@@ -3532,7 +3532,7 @@ static int write_dev_flush(struct btrfs_device *device, int wait)* caller */ device->flush_bio =NULL ; - bio =btrfs_io_bio_alloc (GFP_NOFS ,0 ); + bio =btrfs_io_bio_alloc (0 ); bio->bi_end_io = btrfs_end_empty_barrier; bio->bi_bdev = device->bdev; bio->bi_opf =REQ_OP_WRITE |REQ_SYNC |REQ_PREFLUSH ;
fs/btrfs/extent_io.c +4 -4
@@ -1987,7 +1987,7 @@ int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,ASSERT (!(fs_info->sb->s_flags &MS_RDONLY ));BUG_ON (!mirror_num); - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_iter.bi_size =0 ; map_length = length;
@@ -2331,7 +2331,7 @@ struct bio *btrfs_create_repair_bio(struct inode *inode, struct bio *failed_bio,struct btrfs_io_bio *btrfs_failed_bio;struct btrfs_io_bio *btrfs_bio; - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_end_io = endio_func; bio->bi_iter.bi_sector = failrec->logical >>9 ; bio->bi_bdev = fs_info->fs_devices->latest_bdev;
@@ -2692,12 +2692,12 @@ struct bio *btrfs_bio_clone(struct bio *bio)return new; } -struct bio *btrfs_io_bio_alloc (gfp_t gfp_mask,unsigned int nr_iovecs) +struct bio *btrfs_io_bio_alloc (unsigned int nr_iovecs) {struct bio *bio;/* Bio allocation backed by a bioset does not fail */ - bio =bio_alloc_bioset (gfp_mask, nr_iovecs, btrfs_bioset); + bio =bio_alloc_bioset (GFP_NOFS , nr_iovecs, btrfs_bioset);btrfs_io_bio_init (btrfs_io_bio (bio));return bio; }
fs/btrfs/extent_io.h +1 -1
@@ -463,7 +463,7 @@ void extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,unsigned bits_to_clear, unsigned long page_ops);struct bio *btrfs_bio_alloc (struct block_device *bdev,u64 first_byte); -struct bio *btrfs_io_bio_alloc (gfp_t gfp_mask,unsigned int nr_iovecs); +struct bio *btrfs_io_bio_alloc (unsigned int nr_iovecs);struct bio *btrfs_bio_clone (struct bio *bio);struct bio *btrfs_bio_clone_partial (struct bio *orig,int offset,int size);
fs/btrfs/raid56.c +1 -1
@@ -1098,7 +1098,7 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio, }/* put a new bio on the list */ - bio =btrfs_io_bio_alloc (GFP_NOFS , bio_max_len >>PAGE_SHIFT ?:1 ); + bio =btrfs_io_bio_alloc (bio_max_len >>PAGE_SHIFT ?:1 ); bio->bi_iter.bi_size =0 ; bio->bi_bdev = stripe->dev->bdev; bio->bi_iter.bi_sector = disk_start >>9 ;
fs/btrfs/scrub.c +7 -9
@@ -1737,7 +1737,7 @@ static void scrub_recheck_block(struct btrfs_fs_info *fs_info, }WARN_ON (!page->page); - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_bdev = page->dev->bdev;bio_add_page (bio, page->page,PAGE_SIZE ,0 );
@@ -1825,7 +1825,7 @@ static int scrub_repair_page_from_good_copy(struct scrub_block *sblock_bad,return -EIO ; } - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_bdev = page_bad->dev->bdev; bio->bi_iter.bi_sector = page_bad->physical >>9 ;bio_set_op_attrs (bio,REQ_OP_WRITE ,0 );
@@ -1915,8 +1915,7 @@ static int scrub_add_page_to_wr_bio(struct scrub_ctx *sctx, sbio->dev = sctx->wr_tgtdev; bio = sbio->bio;if (!bio) { - bio =btrfs_io_bio_alloc (GFP_KERNEL , - sctx->pages_per_wr_bio); + bio =btrfs_io_bio_alloc (sctx->pages_per_wr_bio); sbio->bio = bio; }
@@ -2316,8 +2315,7 @@ static int scrub_add_page_to_rd_bio(struct scrub_ctx *sctx, sbio->dev = spage->dev; bio = sbio->bio;if (!bio) { - bio =btrfs_io_bio_alloc (GFP_KERNEL , - sctx->pages_per_rd_bio); + bio =btrfs_io_bio_alloc (sctx->pages_per_rd_bio); sbio->bio = bio; }
@@ -2443,7 +2441,7 @@ static void scrub_missing_raid56_pages(struct scrub_block *sblock) goto bbio_out; } - bio =btrfs_io_bio_alloc (GFP_NOFS ,0 ); + bio =btrfs_io_bio_alloc (0 ); bio->bi_iter.bi_sector = logical >>9 ; bio->bi_private = sblock; bio->bi_end_io = scrub_missing_raid56_end_io;
@@ -3019,7 +3017,7 @@ static void scrub_parity_check_and_repair(struct scrub_parity *sparity)if (ret || !bbio || !bbio->raid_map) goto bbio_out; - bio =btrfs_io_bio_alloc (GFP_NOFS ,0 ); + bio =btrfs_io_bio_alloc (0 ); bio->bi_iter.bi_sector = sparity->logic_start >>9 ; bio->bi_private = sparity; bio->bi_end_io = scrub_parity_bio_endio;
@@ -4626,7 +4624,7 @@ static int write_page_nocow(struct scrub_ctx *sctx, "scrub write_page_nocow(bdev == NULL) is unexpected");return -EIO ; } - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_iter.bi_size =0 ; bio->bi_iter.bi_sector = physical_for_dev_replace >>9 ; bio->bi_bdev = dev->bdev;
Signature · 1 line
2.13.0
Re: [PATCH 2/3] btrfs: add helper to initialize the non-bio part of btrfs_io_bio
In reply to David SterbaQuoted 6 lines · David Sterba
We use
btrfs_biosetfor bios and ask to allocate the entire size ofbtrfs_io_biofrom btrfsbio_alloc_bioset. The member 'bio' is initialized but the bytes from 0 to offset of 'bio' are left uninitialized. Although we initialize some of the members in our helpers, we should initialize the whole structures.
-liubo
Quoted 84 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/extent_io.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)fs/btrfs/extent_io.c +14
-16@@ -2654,22 +2654,28 @@ static void end_bio_extent_readpage(struct bio *bio) }/* +* Initialize the members up to but not including 'bio'. Use after allocating a +* new bio by bio_alloc_bioset as it does not initialize the bytes outside of +* 'bio' because use of __GFP_ZERO is not supported. +*/ +static inline void btrfs_io_bio_init (struct btrfs_io_bio *btrfs_bio) +{ +memset (btrfs_bio,0 , offsetof(struct btrfs_io_bio , bio)); +} + +/* * The following helpers allocate a bio. As it's backed by a bioset, it'll * never fail. We're returning a bio right now but you can call btrfs_io_bio * for the appropriate container_of magic */ struct bio *btrfs_bio_alloc (struct block_device *bdev,u64 first_byte) { -struct btrfs_io_bio *btrfs_bio;struct bio *bio; bio =bio_alloc_bioset (GFP_NOFS ,BIO_MAX_PAGES , btrfs_bioset); bio->bi_bdev = bdev; bio->bi_iter.bi_sector = first_byte >>9 ; - btrfs_bio =btrfs_io_bio (bio); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_io_bio (bio));return bio; }@@ -2681,24 +2687,18 @@ struct bio *btrfs_bio_clone(struct bio *bio)/* Bio allocation backed by a bioset does not fail */ new =bio_clone_fast (bio,GFP_NOFS , btrfs_bioset); btrfs_bio =btrfs_io_bio (new); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_bio); btrfs_bio->iter = bio->bi_iter;return new; }struct bio *btrfs_io_bio_alloc (gfp_t gfp_mask,unsigned int nr_iovecs) { -struct btrfs_io_bio *btrfs_bio;struct bio *bio;/* Bio allocation backed by a bioset does not fail */ bio =bio_alloc_bioset (gfp_mask, nr_iovecs, btrfs_bioset); - btrfs_bio =btrfs_io_bio (bio); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_io_bio (bio));return bio; }@@ -2712,9 +2712,7 @@ struct bio *btrfs_bio_clone_partial(struct bio *orig, int offset, int size)ASSERT (bio); btrfs_bio =btrfs_io_bio (bio); - btrfs_bio->csum =NULL ; - btrfs_bio->csum_allocated =NULL ; - btrfs_bio->end_io =NULL ; +btrfs_io_bio_init (btrfs_bio);bio_trim (bio, offset >>9 , size >>9 ); btrfs_bio->iter = bio->bi_iter;Signature · 1 line
2.13.0
Re: [PATCH 3/3] btrfs: sink gfp parameter to btrfs_io_bio_alloc
In reply to David SterbaQuoted 5 lines · David Sterba
We can hardcode
GFP_NOFStobtrfs_io_bio_alloc, although it means we change it back fromGFP_KERNELin scrub. I'd rather save a few stack bytes from not passing the gfp flags in the remaining, more imporatant, contexts and the bio allocating API now looks more consistent.
-liubo
Quoted 175 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/check-integrity.c | 2 +- fs/btrfs/disk-io.c | 2 +- fs/btrfs/extent_io.c | 8 ++++---- fs/btrfs/extent_io.h | 2 +- fs/btrfs/raid56.c | 2 +- fs/btrfs/scrub.c | 16 +++++++--------- 6 files changed, 15 insertions(+), 17 deletions(-)fs/btrfs/check-integrity.c +1
-1@@ -1638,7 +1638,7 @@ static int btrfsic_read_block(struct btrfsic_state *state,struct bio *bio;unsigned int j; - bio =btrfs_io_bio_alloc (GFP_NOFS , num_pages - i); + bio =btrfs_io_bio_alloc (num_pages - i); bio->bi_bdev = block_ctx->dev->bdev; bio->bi_iter.bi_sector = dev_bytenr >>9 ;bio_set_op_attrs (bio,REQ_OP_READ ,0 );fs/btrfs/disk-io.c +1
-1@@ -3532,7 +3532,7 @@ static int write_dev_flush(struct btrfs_device *device, int wait)* caller */ device->flush_bio =NULL ; - bio =btrfs_io_bio_alloc (GFP_NOFS ,0 ); + bio =btrfs_io_bio_alloc (0 ); bio->bi_end_io = btrfs_end_empty_barrier; bio->bi_bdev = device->bdev; bio->bi_opf =REQ_OP_WRITE |REQ_SYNC |REQ_PREFLUSH ;fs/btrfs/extent_io.c +4
-4@@ -1987,7 +1987,7 @@ int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,ASSERT (!(fs_info->sb->s_flags &MS_RDONLY ));BUG_ON (!mirror_num); - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_iter.bi_size =0 ; map_length = length;@@ -2331,7 +2331,7 @@ struct bio *btrfs_create_repair_bio(struct inode *inode, struct bio *failed_bio,struct btrfs_io_bio *btrfs_failed_bio;struct btrfs_io_bio *btrfs_bio; - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_end_io = endio_func; bio->bi_iter.bi_sector = failrec->logical >>9 ; bio->bi_bdev = fs_info->fs_devices->latest_bdev;@@ -2692,12 +2692,12 @@ struct bio *btrfs_bio_clone(struct bio *bio)return new; } -struct bio *btrfs_io_bio_alloc (gfp_t gfp_mask,unsigned int nr_iovecs) +struct bio *btrfs_io_bio_alloc (unsigned int nr_iovecs) {struct bio *bio;/* Bio allocation backed by a bioset does not fail */ - bio =bio_alloc_bioset (gfp_mask, nr_iovecs, btrfs_bioset); + bio =bio_alloc_bioset (GFP_NOFS , nr_iovecs, btrfs_bioset);btrfs_io_bio_init (btrfs_io_bio (bio));return bio; }fs/btrfs/extent_io.h +1
-1@@ -463,7 +463,7 @@ void extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,unsigned bits_to_clear, unsigned long page_ops);struct bio *btrfs_bio_alloc (struct block_device *bdev,u64 first_byte); -struct bio *btrfs_io_bio_alloc (gfp_t gfp_mask,unsigned int nr_iovecs); +struct bio *btrfs_io_bio_alloc (unsigned int nr_iovecs);struct bio *btrfs_bio_clone (struct bio *bio);struct bio *btrfs_bio_clone_partial (struct bio *orig,int offset,int size);fs/btrfs/raid56.c +1
-1@@ -1098,7 +1098,7 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio, }/* put a new bio on the list */ - bio =btrfs_io_bio_alloc (GFP_NOFS , bio_max_len >>PAGE_SHIFT ?:1 ); + bio =btrfs_io_bio_alloc (bio_max_len >>PAGE_SHIFT ?:1 ); bio->bi_iter.bi_size =0 ; bio->bi_bdev = stripe->dev->bdev; bio->bi_iter.bi_sector = disk_start >>9 ;fs/btrfs/scrub.c +7
-9@@ -1737,7 +1737,7 @@ static void scrub_recheck_block(struct btrfs_fs_info *fs_info, }WARN_ON (!page->page); - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_bdev = page->dev->bdev;bio_add_page (bio, page->page,PAGE_SIZE ,0 );@@ -1825,7 +1825,7 @@ static int scrub_repair_page_from_good_copy(struct scrub_block *sblock_bad,return -EIO ; } - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_bdev = page_bad->dev->bdev; bio->bi_iter.bi_sector = page_bad->physical >>9 ;bio_set_op_attrs (bio,REQ_OP_WRITE ,0 );@@ -1915,8 +1915,7 @@ static int scrub_add_page_to_wr_bio(struct scrub_ctx *sctx, sbio->dev = sctx->wr_tgtdev; bio = sbio->bio;if (!bio) { - bio =btrfs_io_bio_alloc (GFP_KERNEL , - sctx->pages_per_wr_bio); + bio =btrfs_io_bio_alloc (sctx->pages_per_wr_bio); sbio->bio = bio; }@@ -2316,8 +2315,7 @@ static int scrub_add_page_to_rd_bio(struct scrub_ctx *sctx, sbio->dev = spage->dev; bio = sbio->bio;if (!bio) { - bio =btrfs_io_bio_alloc (GFP_KERNEL , - sctx->pages_per_rd_bio); + bio =btrfs_io_bio_alloc (sctx->pages_per_rd_bio); sbio->bio = bio; }@@ -2443,7 +2441,7 @@ static void scrub_missing_raid56_pages(struct scrub_block *sblock) goto bbio_out; } - bio =btrfs_io_bio_alloc (GFP_NOFS ,0 ); + bio =btrfs_io_bio_alloc (0 ); bio->bi_iter.bi_sector = logical >>9 ; bio->bi_private = sblock; bio->bi_end_io = scrub_missing_raid56_end_io;@@ -3019,7 +3017,7 @@ static void scrub_parity_check_and_repair(struct scrub_parity *sparity)if (ret || !bbio || !bbio->raid_map) goto bbio_out; - bio =btrfs_io_bio_alloc (GFP_NOFS ,0 ); + bio =btrfs_io_bio_alloc (0 ); bio->bi_iter.bi_sector = sparity->logic_start >>9 ; bio->bi_private = sparity; bio->bi_end_io = scrub_parity_bio_endio;@@ -4626,7 +4624,7 @@ static int write_page_nocow(struct scrub_ctx *sctx, "scrub write_page_nocow(bdev == NULL) is unexpected");return -EIO ; } - bio =btrfs_io_bio_alloc (GFP_NOFS ,1 ); + bio =btrfs_io_bio_alloc (1 ); bio->bi_iter.bi_size =0 ; bio->bi_iter.bi_sector = physical_for_dev_replace >>9 ; bio->bi_bdev = dev->bdev;Signature · 6 lines
2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html