| Message ID | 0177d54fd7d5d9e96ee0bcdc29facd1324149a0e.1621351444.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: zoned: fix writes on a compressed zoned filesystem | expand |
On 2021/5/18 下午11:40, Johannes Thumshirn wrote: > To be able to construct a zone append bio we need to look up the > btrfs_device. The code doing the chunk map lookup to get the device is > present in btrfs_submit_compressed_write and submit_extent_page. > > Factor out the lookup calls into a helper and use it in the submission > paths. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/compression.c | 16 ++++------------ > fs/btrfs/extent_io.c | 16 +++++----------- > fs/btrfs/zoned.c | 21 +++++++++++++++++++++ > fs/btrfs/zoned.h | 9 +++++++++ > 4 files changed, 39 insertions(+), 23 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index f224c35de5d8..b101bc483e40 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -428,24 +428,16 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start, > bio->bi_end_io = end_compressed_bio_write; > > if (use_append) { > - struct extent_map *em; > - struct map_lookup *map; > - struct block_device *bdev; > + struct btrfs_device *device; > > - em = btrfs_get_chunk_map(fs_info, disk_start, PAGE_SIZE); > - if (IS_ERR(em)) { > + device = btrfs_zoned_get_device(fs_info, disk_start, PAGE_SIZE); Not sure if it's too late, but I find that all callers are just passing PAGE_SIZE (to be more accurate, should be sectorsize) to btrfs_zoned_get_device(). Do we really need that @length parameter anyway? Since we're just using one sector to grab the chunk map, the @length parameter is completely useless and we can grab sectorsize using fs_info. This makes me to dig deeper than needed every time I see such length usage... Thanks, Qu > + if (IS_ERR(device)) { > kfree(cb); > bio_put(bio); > return BLK_STS_NOTSUPP; > } > > - map = em->map_lookup; > - /* We only support single profile for now */ > - ASSERT(map->num_stripes == 1); > - bdev = map->stripes[0].dev->bdev; > - > - bio_set_dev(bio, bdev); > - free_extent_map(em); > + bio_set_dev(bio, device->bdev); > } > > if (blkcg_css) { > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index ce6364dd1517..2b250c610562 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -3266,19 +3266,13 @@ static int submit_extent_page(unsigned int opf, > wbc_account_cgroup_owner(wbc, page, io_size); > } > if (btrfs_is_zoned(fs_info) && bio_op(bio) == REQ_OP_ZONE_APPEND) { > - struct extent_map *em; > - struct map_lookup *map; > + struct btrfs_device *device; > > - em = btrfs_get_chunk_map(fs_info, disk_bytenr, io_size); > - if (IS_ERR(em)) > - return PTR_ERR(em); > + device = btrfs_zoned_get_device(fs_info, disk_bytenr, io_size); > + if (IS_ERR(device)) > + return PTR_ERR(device); > > - map = em->map_lookup; > - /* We only support single profile for now */ > - ASSERT(map->num_stripes == 1); > - btrfs_io_bio(bio)->device = map->stripes[0].dev; > - > - free_extent_map(em); > + btrfs_io_bio(bio)->device = device; > } > > *bio_ret = bio; > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c > index b9d5579a578d..15843a858bf6 100644 > --- a/fs/btrfs/zoned.c > +++ b/fs/btrfs/zoned.c > @@ -1520,3 +1520,24 @@ int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, u64 logical, > length = wp - physical_pos; > return btrfs_zoned_issue_zeroout(tgt_dev, physical_pos, length); > } > + > +struct btrfs_device *btrfs_zoned_get_device(struct btrfs_fs_info *fs_info, > + u64 logical, u64 length) > +{ > + struct btrfs_device *device; > + struct extent_map *em; > + struct map_lookup *map; > + > + em = btrfs_get_chunk_map(fs_info, logical, length); > + if (IS_ERR(em)) > + return ERR_CAST(em); > + > + map = em->map_lookup; > + /* We only support single profile for now */ > + ASSERT(map->num_stripes == 1); > + device = map->stripes[0].dev; > + > + free_extent_map(em); > + > + return device; > +} > diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h > index e55d32595c2c..b0ae2608cb6b 100644 > --- a/fs/btrfs/zoned.h > +++ b/fs/btrfs/zoned.h > @@ -65,6 +65,8 @@ void btrfs_revert_meta_write_pointer(struct btrfs_block_group *cache, > int btrfs_zoned_issue_zeroout(struct btrfs_device *device, u64 physical, u64 length); > int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, u64 logical, > u64 physical_start, u64 physical_pos); > +struct btrfs_device *btrfs_zoned_get_device(struct btrfs_fs_info *fs_info, > + u64 logical, u64 length); > #else /* CONFIG_BLK_DEV_ZONED */ > static inline int btrfs_get_dev_zone(struct btrfs_device *device, u64 pos, > struct blk_zone *zone) > @@ -191,6 +193,13 @@ static inline int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, > return -EOPNOTSUPP; > } > > +static inline struct btrfs_device *btrfs_zoned_get_device( > + struct btrfs_fs_info *fs_info, > + u64 logical, u64 length) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} > + > #endif > > static inline bool btrfs_dev_is_sequential(struct btrfs_device *device, u64 pos) >
On 24/05/2021 12:01, Qu Wenruo wrote: >> if (use_append) { >> - struct extent_map *em; >> - struct map_lookup *map; >> - struct block_device *bdev; >> + struct btrfs_device *device; >> >> - em = btrfs_get_chunk_map(fs_info, disk_start, PAGE_SIZE); >> - if (IS_ERR(em)) { >> + device = btrfs_zoned_get_device(fs_info, disk_start, PAGE_SIZE); > Not sure if it's too late, but I find that all callers are just passing > PAGE_SIZE (to be more accurate, should be sectorsize) to > btrfs_zoned_get_device(). > > Do we really need that @length parameter anyway? > > Since we're just using one sector to grab the chunk map, the @length > parameter is completely useless and we can grab sectorsize using fs_info. > > This makes me to dig deeper than needed every time I see such length > usage... Hm yes I think you're right. I'll send a cleanup soon.
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index f224c35de5d8..b101bc483e40 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -428,24 +428,16 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start, bio->bi_end_io = end_compressed_bio_write; if (use_append) { - struct extent_map *em; - struct map_lookup *map; - struct block_device *bdev; + struct btrfs_device *device; - em = btrfs_get_chunk_map(fs_info, disk_start, PAGE_SIZE); - if (IS_ERR(em)) { + device = btrfs_zoned_get_device(fs_info, disk_start, PAGE_SIZE); + if (IS_ERR(device)) { kfree(cb); bio_put(bio); return BLK_STS_NOTSUPP; } - map = em->map_lookup; - /* We only support single profile for now */ - ASSERT(map->num_stripes == 1); - bdev = map->stripes[0].dev->bdev; - - bio_set_dev(bio, bdev); - free_extent_map(em); + bio_set_dev(bio, device->bdev); } if (blkcg_css) { diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index ce6364dd1517..2b250c610562 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3266,19 +3266,13 @@ static int submit_extent_page(unsigned int opf, wbc_account_cgroup_owner(wbc, page, io_size); } if (btrfs_is_zoned(fs_info) && bio_op(bio) == REQ_OP_ZONE_APPEND) { - struct extent_map *em; - struct map_lookup *map; + struct btrfs_device *device; - em = btrfs_get_chunk_map(fs_info, disk_bytenr, io_size); - if (IS_ERR(em)) - return PTR_ERR(em); + device = btrfs_zoned_get_device(fs_info, disk_bytenr, io_size); + if (IS_ERR(device)) + return PTR_ERR(device); - map = em->map_lookup; - /* We only support single profile for now */ - ASSERT(map->num_stripes == 1); - btrfs_io_bio(bio)->device = map->stripes[0].dev; - - free_extent_map(em); + btrfs_io_bio(bio)->device = device; } *bio_ret = bio; diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index b9d5579a578d..15843a858bf6 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -1520,3 +1520,24 @@ int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, u64 logical, length = wp - physical_pos; return btrfs_zoned_issue_zeroout(tgt_dev, physical_pos, length); } + +struct btrfs_device *btrfs_zoned_get_device(struct btrfs_fs_info *fs_info, + u64 logical, u64 length) +{ + struct btrfs_device *device; + struct extent_map *em; + struct map_lookup *map; + + em = btrfs_get_chunk_map(fs_info, logical, length); + if (IS_ERR(em)) + return ERR_CAST(em); + + map = em->map_lookup; + /* We only support single profile for now */ + ASSERT(map->num_stripes == 1); + device = map->stripes[0].dev; + + free_extent_map(em); + + return device; +} diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h index e55d32595c2c..b0ae2608cb6b 100644 --- a/fs/btrfs/zoned.h +++ b/fs/btrfs/zoned.h @@ -65,6 +65,8 @@ void btrfs_revert_meta_write_pointer(struct btrfs_block_group *cache, int btrfs_zoned_issue_zeroout(struct btrfs_device *device, u64 physical, u64 length); int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, u64 logical, u64 physical_start, u64 physical_pos); +struct btrfs_device *btrfs_zoned_get_device(struct btrfs_fs_info *fs_info, + u64 logical, u64 length); #else /* CONFIG_BLK_DEV_ZONED */ static inline int btrfs_get_dev_zone(struct btrfs_device *device, u64 pos, struct blk_zone *zone) @@ -191,6 +193,13 @@ static inline int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, return -EOPNOTSUPP; } +static inline struct btrfs_device *btrfs_zoned_get_device( + struct btrfs_fs_info *fs_info, + u64 logical, u64 length) +{ + return ERR_PTR(-EOPNOTSUPP); +} + #endif static inline bool btrfs_dev_is_sequential(struct btrfs_device *device, u64 pos)
To be able to construct a zone append bio we need to look up the btrfs_device. The code doing the chunk map lookup to get the device is present in btrfs_submit_compressed_write and submit_extent_page. Factor out the lookup calls into a helper and use it in the submission paths. Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- fs/btrfs/compression.c | 16 ++++------------ fs/btrfs/extent_io.c | 16 +++++----------- fs/btrfs/zoned.c | 21 +++++++++++++++++++++ fs/btrfs/zoned.h | 9 +++++++++ 4 files changed, 39 insertions(+), 23 deletions(-)