| Message ID | 20230220115926.54623-2-jwd@defmacro.it |
|---|---|
| State | New |
| Headers | show |
| Series | Support Flexible Data Placement (FDP) | expand |
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 >
On Feb 24 09:20, Joel Granados wrote: > 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? > It is useful because the Endurance Group Information log page wants to report Data Units Read/Written in billions of bytes, not in sectors. But yes, the commit message should be update since it is no longer related to OCP.
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);