Re: [RFC PATCH v4 1/3] bcache: export bcache zone information for zoned backing device

[Damien Le Moal <Damien.LeMoal@wdc.com>]

On 2020/06/02 21:50, Coly Li wrote:
>>>         /* convert back to LBA of the bcache device*/
>>>         zone->start -= data_offset;
>>>         if (zone->cond != BLK_ZONE_COND_NOT_WP)
>>>                 zone->wp -= data_offset;
>>
>> Since you have the BLK_ZONE_COND_NOT_WP condition in the switch-case,
>> this is not needed. 
>>
>>>
>>>         switch (zone->cond) {
>>>         case BLK_ZONE_COND_NOT_WP:
>>>                 /* No write pointer available */
>>>                 break;
>>>         case BLK_ZONE_COND_EMPTY:
>>
>> zone->wp = zone->start;
>>
> 
> Correct me if I am wrong. I assume when the zone is in empty condition,
> LBA of zone->wp should be exactly equal to zone->start before bcache
> does the conversion. Therefore 'zone->wp - data_offset' should still be
> equal to 'zone->start - data_offset'. Therefore explicitly handle it in
> 'case BLK_ZONE_COND_EMPTY:' should be equal to handle it in 'default:' part.

Yes, for an empty zone, the zone wp is always equal to the start sector of the
zone. So yes, for an empty zone, wp - data_offset is correct. But it is not
necessary since you already need to do zone->start -= data_offset, all you need
to do for an empty zone is: zone->wp = zone->start. That is less arithmetic (no
need for the subtraction), so is faster :)

This code you have:

	if (zone->cond != BLK_ZONE_COND_NOT_WP)
		zone->wp -= data_offset;

will indeed lead to the correct result for an empty zone, but as I explained, it
will also do the subtraction for zone conditions with an undefined value. No
need to do all this. A switch case handling all conditions makes things very clear.

-- 
Damien Le Moal
Western Digital Research