Re: [PATCH v3 1/5] hw/nvme: move adjustment of data_units{read,written}

[Joel Granados <j.granados@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2993 bytes --]

On Mon, Feb 20, 2023 at 12:59:22PM +0100, Jesper Devantier wrote:
> From: Joel Granados <j.granados@samsung.com>
> 
> In order to return the units_{read/written} required by the SMART log we
> need to shift the number of bytes value by BDRV_SECTORS_BITS and multiply
> by 1000. This is a prep patch that moves this adjustment to where the SMART
> log is calculated in order to use the stats struct for calculating OCP
> extended smart log values.

This was originally part of another patchset that looked at adding OCP
to qemu. Why is it needed for this patch set? Should we change the
wording of the commit to remove the OCP specific stuff? or should we
just remove the patch all together?

best
Joel

> 
> Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/nvme/ctrl.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index f25cc2c235..99b92ff20b 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -4386,8 +4386,8 @@ static void nvme_set_blk_stats(NvmeNamespace *ns, struct nvme_stats *stats)
>  {
>      BlockAcctStats *s = blk_get_stats(ns->blkconf.blk);
>  
> -    stats->units_read += s->nr_bytes[BLOCK_ACCT_READ] >> BDRV_SECTOR_BITS;
> -    stats->units_written += s->nr_bytes[BLOCK_ACCT_WRITE] >> BDRV_SECTOR_BITS;
> +    stats->units_read += s->nr_bytes[BLOCK_ACCT_READ];
> +    stats->units_written += s->nr_bytes[BLOCK_ACCT_WRITE];
>      stats->read_commands += s->nr_ops[BLOCK_ACCT_READ];
>      stats->write_commands += s->nr_ops[BLOCK_ACCT_WRITE];
>  }
> @@ -4401,6 +4401,7 @@ static uint16_t nvme_smart_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len,
>      uint32_t trans_len;
>      NvmeNamespace *ns;
>      time_t current_ms;
> +    uint64_t u_read, u_written;
>  
>      if (off >= sizeof(smart)) {
>          return NVME_INVALID_FIELD | NVME_DNR;
> @@ -4427,10 +4428,11 @@ static uint16_t nvme_smart_info(NvmeCtrl *n, uint8_t rae, uint32_t buf_len,
>      trans_len = MIN(sizeof(smart) - off, buf_len);
>      smart.critical_warning = n->smart_critical_warning;
>  
> -    smart.data_units_read[0] = cpu_to_le64(DIV_ROUND_UP(stats.units_read,
> -                                                        1000));
> -    smart.data_units_written[0] = cpu_to_le64(DIV_ROUND_UP(stats.units_written,
> -                                                           1000));
> +    u_read = DIV_ROUND_UP(stats.units_read >> BDRV_SECTOR_BITS, 1000);
> +    u_written = DIV_ROUND_UP(stats.units_written >> BDRV_SECTOR_BITS, 1000);
> +
> +    smart.data_units_read[0] = cpu_to_le64(u_read);
> +    smart.data_units_written[0] = cpu_to_le64(u_written);
>      smart.host_read_commands[0] = cpu_to_le64(stats.read_commands);
>      smart.host_write_commands[0] = cpu_to_le64(stats.write_commands);
>  
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]