On 2021/01/28 14:38, Chaitanya Kulkarni wrote:
> On 1/27/21 20:47, Damien Le Moal wrote:
>> -void sd_zbc_release_disk(struct scsi_disk *sdkp)
>> +static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
>> {
>> + /* Serialize against revalidate zones */
>> + mutex_lock(&sdkp->rev_mutex);
>> +
>> kvfree(sdkp->zones_wp_offset);
>> sdkp->zones_wp_offset = NULL;
>> kfree(sdkp->zone_wp_update_buf);
>> sdkp->zone_wp_update_buf = NULL;
>> +
>> + sdkp->nr_zones = 0;
>> + sdkp->rev_nr_zones = 0;
>> + sdkp->zone_blocks = 0;
>> + sdkp->rev_zone_blocks = 0;
>> +
>> + mutex_unlock(&sdkp->rev_mutex);
>> +}
>> +
>> +void sd_zbc_release_disk(struct scsi_disk *sdkp)
>> +{
>> + if (sd_is_zoned(sdkp))
>> + sd_zbc_clear_zone_info(sdkp);
>> }
>>
> If I'm not wrong there is only one caller for sd_zbc_clear_zone_info().
> Is there any reason why sd_zbc_clear_zone_info() is notopen coded with
> a meaningful comment in sd_zbc_release_disk() ? e.g. :-
There are 2 call sites: sd_zbc_read_zones() and sd_zbc_release_disk().
>
> void sd_zbc_release_disk(struct scsi_disk *sdkp)
> {
> if (!sd_is_zoned(sdkp))
> return;
> /* Serialize against revalidate zones */
> mutex_lock(&sdkp->rev_mutex);
>
> kvfree(sdkp->zones_wp_offset);
> sdkp->zones_wp_offset = NULL;
> kfree(sdkp->zone_wp_update_buf);
> sdkp->zone_wp_update_buf = NULL;
>
> /* clear zone info */
> sdkp->nr_zones = 0;
> sdkp->rev_nr_zones = 0;
> sdkp->zone_blocks = 0;
> sdkp->rev_zone_blocks = 0;
>
> mutex_unlock(&sdkp->rev_mutex);
> }
>
>
> unless I miss something, in either case LGTM.
>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
>
>
--
Damien Le Moal
Western Digital Research
