Re: [PATCH v4 8/8] sd_zbc: clear zone resources for non-zoned case

On 2021/01/28 14:38, Chaitanya Kulkarni wrote:

On 1/27/21 20:47, Damien Le Moal wrote:

quoted

-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 <redacted>

-- 
Damien Le Moal
Western Digital Research