Re: [PATCH v2 1/8] zbd: refer file->last_start[] instead of sectors with data accounting

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Tue, Feb 07, 2023 at 03:37:32PM +0900, Shin'ichiro Kawasaki wrote:
> To decide the first IO direction of randrw workload, the function
> zbd_adjust_ddir() refers to the zbd_info->sectors_with_data value which
> indicates the number of bytes written to the zoned block devices being
> accessed. However, this accounting has two issues. The first issue is
> wrong accounting for multiple jobs with different write ranges. The
> second issue is job start up failure due to zone lock contention.
> 
> Avoid using zbd_info->sectors_with_data and simply refer to file->
> last_start[DDIR_WRITE] instead. It is initialized with -1ULL for each
> job. After any write operation is done by the job, it keeps valid
> offset. If it has valid offset, written data is expected and the first
> IO direction can be read.
> 
> Also remove zbd_info->sectors_with_data, which is no longer used. Keep
> the field zbd_info->wp_sectors_with_data since it is still used for
> zones with write pointers.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  zbd.c | 14 +++-----------
>  zbd.h |  2 --
>  2 files changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/zbd.c b/zbd.c
> index d1e469f6..f5e76c40 100644
> --- a/zbd.c
> +++ b/zbd.c
> @@ -286,7 +286,6 @@ static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
>  	}
>  
>  	pthread_mutex_lock(&f->zbd_info->mutex);
> -	f->zbd_info->sectors_with_data -= data_in_zone;
>  	f->zbd_info->wp_sectors_with_data -= data_in_zone;
>  	pthread_mutex_unlock(&f->zbd_info->mutex);
>  
> @@ -1201,7 +1200,6 @@ static uint64_t zbd_process_swd(struct thread_data *td,
>  				const struct fio_file *f, enum swd_action a)
>  {
>  	struct fio_zone_info *zb, *ze, *z;
> -	uint64_t swd = 0;
>  	uint64_t wp_swd = 0;
>  
>  	zb = zbd_get_zone(f, f->min_zone);
> @@ -1211,17 +1209,14 @@ static uint64_t zbd_process_swd(struct thread_data *td,
>  			zone_lock(td, f, z);
>  			wp_swd += z->wp - z->start;
>  		}
> -		swd += z->wp - z->start;
>  	}
>  
>  	pthread_mutex_lock(&f->zbd_info->mutex);
>  	switch (a) {
>  	case CHECK_SWD:
> -		assert(f->zbd_info->sectors_with_data == swd);
>  		assert(f->zbd_info->wp_sectors_with_data == wp_swd);
>  		break;
>  	case SET_SWD:
> -		f->zbd_info->sectors_with_data = swd;
>  		f->zbd_info->wp_sectors_with_data = wp_swd;
>  		break;
>  	}
> @@ -1231,7 +1226,7 @@ static uint64_t zbd_process_swd(struct thread_data *td,
>  		if (z->has_wp)
>  			zone_unlock(z);
>  
> -	return swd;
> +	return wp_swd;
>  }
>  
>  /*
> @@ -1640,10 +1635,8 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
>  		 * have occurred.
>  		 */
>  		pthread_mutex_lock(&zbd_info->mutex);
> -		if (z->wp <= zone_end) {
> -			zbd_info->sectors_with_data += zone_end - z->wp;
> +		if (z->wp <= zone_end)
>  			zbd_info->wp_sectors_with_data += zone_end - z->wp;
> -		}
>  		pthread_mutex_unlock(&zbd_info->mutex);
>  		z->wp = zone_end;
>  		break;
> @@ -1801,8 +1794,7 @@ enum fio_ddir zbd_adjust_ddir(struct thread_data *td, struct io_u *io_u,
>  	if (ddir != DDIR_READ || !td_rw(td))
>  		return ddir;
>  
> -	if (io_u->file->zbd_info->sectors_with_data ||
> -	    td->o.read_beyond_wp)
> +	if (io_u->file->last_start[DDIR_WRITE] != -1ULL || td->o.read_beyond_wp)
>  		return DDIR_READ;
>  
>  	return DDIR_WRITE;
> diff --git a/zbd.h b/zbd.h
> index d425707e..9ab25c47 100644
> --- a/zbd.h
> +++ b/zbd.h
> @@ -54,7 +54,6 @@ struct fio_zone_info {
>   * @mutex: Protects the modifiable members in this structure (refcount and
>   *		num_open_zones).
>   * @zone_size: size of a single zone in bytes.
> - * @sectors_with_data: total size of data in all zones in units of 512 bytes
>   * @wp_sectors_with_data: total size of data in zones with write pointers in
>   *                        units of 512 bytes
>   * @zone_size_log2: log2 of the zone size in bytes if it is a power of 2 or 0
> @@ -76,7 +75,6 @@ struct zoned_block_device_info {
>  	uint32_t		max_open_zones;
>  	pthread_mutex_t		mutex;
>  	uint64_t		zone_size;
> -	uint64_t		sectors_with_data;
>  	uint64_t		wp_sectors_with_data;
>  	uint32_t		zone_size_log2;
>  	uint32_t		nr_zones;
> -- 
> 2.38.1
> 

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>