On Mon, Dec 18, 2023 at 08:21:22AM +0000, 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. Both functions are used for iterate_devices's callback in dm_table_supports_zoned_model(). As shown in raid_iterate_devices(), iterate_devices() returns 0 if the callback func calls on all the devices returns 0, or returns a non-zero result early otherwise. So, the iterate_devices() call returns "true" if any one of the underlying devices is (zoned|not zoned). Since we cannot create lambda as in other fancy languages, we need two functions...