On 12/18/23 17:21, Ed Tsai (蔡宗軒) wrote: > On Mon, 2023-12-18 at 15:53 +0900, Damien Le Moal wrote: >> On 2023/12/18 15:15, Ed Tsai (蔡宗軒) wrote: >>> Hi Christoph, >>> >>> some minor suggestions: >>> >>> On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote: >>>> >>>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c >>>> index 198d38b53322c1..260b5b8f2b0d7e 100644 >>>> --- a/drivers/md/dm-table.c >>>> +++ b/drivers/md/dm-table.c >>>> @@ -1579,21 +1579,18 @@ bool dm_table_has_no_data_devices(struct >>>> dm_table *t) >>>> return true; >>>> } >>>> >>>> -static int device_not_zoned_model(struct dm_target *ti, struct >>>> dm_dev *dev, >>>> - sector_t start, sector_t len, void >>>> *data) >>>> +static int device_not_zoned(struct dm_target *ti, struct dm_dev >>>> *dev, >>>> + sector_t start, sector_t len, void *data) >>>> { >>>> -struct request_queue *q = bdev_get_queue(dev->bdev); >>>> -enum blk_zoned_model *zoned_model = data; >>>> +bool *zoned = data; >>>> >>>> -return blk_queue_zoned_model(q) != *zoned_model; >>>> +return bdev_is_zoned(dev->bdev) != *zoned; >>>> } >>>> >>>> static int device_is_zoned_model(struct dm_target *ti, struct >> dm_dev >>>> *dev, >>>> sector_t start, sector_t len, void >>>> *data) >>> >>> Seems like the word "model" should also be remove here. >>> >>>> { >>>> -struct request_queue *q = bdev_get_queue(dev->bdev); >>>> - >>>> -return blk_queue_zoned_model(q) != BLK_ZONED_NONE; >>>> +return bdev_is_zoned(dev->bdev); >>>> } >>>> >>>> /* >>>> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct >>>> dm_target *ti, struct dm_dev *dev, >>>> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the devices >> can >>>> have any >>>> * zoned model with all zoned devices having the same zone size. >>>> */ >>>> -static bool dm_table_supports_zoned_model(struct dm_table *t, >>>> - enum blk_zoned_model >>>> zoned_model) >>>> +static bool dm_table_supports_zoned(struct dm_table *t, bool >> zoned) >>>> { >>>> for (unsigned int i = 0; i < t->num_targets; i++) { >>>> struct dm_target *ti = dm_table_get_target(t, i); >>>> @@ -1623,11 +1619,11 @@ static bool >>>> dm_table_supports_zoned_model(struct dm_table *t, >>>> >>>> if (dm_target_supports_zoned_hm(ti->type)) { >>>> if (!ti->type->iterate_devices || >>>> - ti->type->iterate_devices(ti, >>>> device_not_zoned_model, >>>> - &zoned_model)) >>>> + ti->type->iterate_devices(ti, >>>> device_not_zoned, >>>> + &zoned)) >>>> return false; >>>> } else if (!dm_target_supports_mixed_zoned_model(ti- >>>>> type)) { >>>> -if (zoned_model == BLK_ZONED_HM) >>>> +if (zoned) >>>> return false; >>>> } >>>> } >>> >>> The parameter "bool zoned" is redundant. It should be removed from >> the >>> above 3 functions > > The two func, is zoned and not zoned, are essentially the same. They > can be simplified into one function. Maybe... But that needs testing/checking. I added the one because I could not reuse the other given what is being tested. > >>> >>> Additionally, because we no longer need to distinguish the zoned >> model >>> here, DM_TARGET_MIXED_ZONED_MODEL is meaningless. We can also clean >> up >>> its related code. >> >> Nope. The mixed thing is for mixing up non-zoned with zoned models. >> For the entire DM code, HM and HA are both treated as HM-like zoned. >> >> -- >> Damien Le Moal >> Western Digital Research > > Thank you. I have some misunderstanding. Please disregard it. -- Damien Le Moal Western Digital Research