Hi Damien,
>> +static int dm_po2z_map_read_emulated_area(struct dm_po2z_target *dmh,
>> + struct bio *bio)
>
> This really should be called dm_po2z_handle_read() since it handles both cases
> of read commands, with and without the accept partial. Note that this is
> retesting the need for a split after that was already tested in dm_po2z_map()
> with bio_across_emulated_zone_area(). So the code should be better organized to
> avoid that repetition.
>
> You can simplify the code by having bio_across_emulated_zone_area() return 0 for
> a bio that does not cross the zone capacity and return the number of sectors up
> to the zone capacity from the bio start if there is a cross. That value can then
The offset will also be zero if the BIO starts at the zone capacity and
dm_po2z_map will assume that the bio did not cross the emulated zone boundary.
> be used to call dm_accept_partial_bio() directly in dm_po2z_map() for read
> commands. That will make this function much simpler and essentially turn it into
> dm_po2z_read_zeroes().
>
>> +{
What about something like this? We have one function:
bio_in_emulated_zone_area() that checks if a BIO is partly or completely in the
emulated zone area and also returns the offset from bio to the emulated zone
boundary (zone capacity of the device).
/**
* bio_in_emulated_zone_area - check if bio is in the emulated zone area
* @dmh: pozone target data
* @bio: bio
* @offset: bio offset to emulated zone boundary
*
* Check if a @bio is partly or completely in the emulated zone area. If the
* @bio is partly in the emulated zone area, @offset
* can be used to split the @bio across the emulated zone boundary. @offset
* will be negative if the @bio completely lies in the emulated area.
* */
static bool bio_in_emulated_zone_area(struct dm_po2z_target *dmh,
struct bio *bio, int *offset)
{
unsigned int zone_idx = po2_zone_no(dmh, bio->bi_iter.bi_sector);
sector_t nr_sectors = bio->bi_iter.bi_size >> SECTOR_SHIFT;
sector_t relative_sect_in_zone =
bio->bi_iter.bi_sector - (zone_idx << dmh->zone_size_po2_shift);
*offset = dmh->zone_size - relative_sect_in_zone;
return (relative_sect_in_zone + nr_sectors) > dmh->zone_size;
}
static int dm_po2z_read_zeroes(struct bio *bio)
{
zero_fill_bio(bio);
bio_endio(bio);
return DM_MAPIO_SUBMITTED;
}
static int dm_po2z_remap_sector(struct dm_po2z_target *dmh, struct bio *bio)
{
bio->bi_iter.bi_sector =
target_to_device_sect(dmh, bio->bi_iter.bi_sector);
return DM_MAPIO_REMAPPED;
}
static int dm_po2z_map(struct dm_target *ti, struct bio *bio)
{
struct dm_po2z_target *dmh = ti->private;
bio_set_dev(bio, dmh->dev->bdev);
if (bio_sectors(bio) || op_is_zone_mgmt(bio_op(bio))) {
int split_io_pos;
if (!bio_in_emulated_zone_area(dmh, bio, &split_io_pos)) {
return dm_po2z_remap_sector(dmh, bio);
}
/*
* Read operation on the emulated zone area (between zone capacity
* and zone size) will fill the bio with zeroes. Any other operation
* in the emulated area should return an error.
*/
if (bio_op(bio) == REQ_OP_READ) {
if (split_io_pos > 0) {
dm_accept_partial_bio(bio, split_io_pos);
return dm_po2z_remap_sector(dmh, bio);
}
return dm_po2z_read_zeroes(bio);
}
return DM_MAPIO_KILL;
}
return DM_MAPIO_REMAPPED;
}
Let me know what you think.
