Re: [PATCH 3/5] block: remove support for the host aware zone model

[Damien Le Moal <dlemoal@kernel.org>]

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