[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 0/2] Add OCP extended log to nvme QEMU
From: |
Joel Granados |
Subject: |
Re: [PATCH v4 0/2] Add OCP extended log to nvme QEMU |
Date: |
Thu, 8 Dec 2022 17:09:31 +0100 |
ping.
Is the solution to the guid constant ok?
Best
On Fri, Nov 25, 2022 at 10:48:06AM +0100, Joel Granados wrote:
> The motivation and description are contained in the last patch in this set.
> Will copy paste it here for convenience:
>
> In order to evaluate write amplification factor (WAF) within the storage
> stack it is important to know the number of bytes written to the
> controller. The existing SMART log value of Data Units Written is too
> coarse (given in units of 500 Kb) and so we add the SMART health
> information extended from the OCP specification (given in units of bytes).
>
> To accommodate different vendor specific specifications like OCP, we add a
> multiplexing function (nvme_vendor_specific_log) which will route to the
> different log functions based on arguments and log ids. We only return the
> OCP extended smart log when the command is 0xC0 and ocp has been turned on
> in the args.
>
> Though we add the whole nvme smart log extended structure, we only
> populate
> the physical_media_units_{read,written}, log_page_version and
> log_page_uuid.
>
> V4 changes:
> 1. Fixed cpu_to_le64 instead of cpu_to_le32
> 2. Variable naming : uuid -> guid
> 3. Changed how the guid value appears in the code:
> Used to be:
> smart_l.log_page_uuid[0] = 0xA4F2BFEA2810AFC5;
> smart_l.log_page_uuid[1] = 0xAFD514C97C6F4F9C;
>
> Now is:
> static const uint8_t guid[16] = {
> 0xC5, 0xAF, 0x10, 0x28, 0xEA, 0xBF, 0xF2, 0xA4,
> 0x9C, 0x4F, 0x6F, 0x7C, 0xC9, 0x14, 0xD5, 0xAF
> };
>
> This is different from what @klaus suggested because I want to keep it
> consistent to what nvme-cli currently implements. I think here we can
> either change both nvme-cli and this patch or leave the order of the
> bytes as they are here. This all depends on how you interpret the Spec
> (which is ambiguous)
>
> V3 changes:
> 1. Corrected a bunch of checkpatch issues. Since I changed the first patch
> I did not include the reviewed-by.
> 2. Included some documentation in nvme.rst for the ocp argument
> 3. Squashed the ocp arg changes into the main patch.
> 4. Fixed several comments and an open parenthesis
> 5. Hex values are now in lower case.
> 6. Change the reserved format to rsvd<BYTEOFFSET>
> 7. Made sure that NvmeCtrl is the first arg in all the functions.
> 8. Fixed comment on commit of main patch
>
> V2 changes:
> 1. I moved the ocp parameter from the namespace to the subsystem as it is
> defined there in the OCP specification
> 2. I now accumulate statistics from all namespaces and report them back on
> the extended log as per the spec.
> 3. I removed the default case in the switch in nvme_vendor_specific_log as
> it does not have any special function.
>
> Joel Granados (2):
> nvme: Move adjustment of data_units{read,written}
> nvme: Add physical writes/reads from OCP log
>
> docs/system/devices/nvme.rst | 7 ++++
> hw/nvme/ctrl.c | 73 +++++++++++++++++++++++++++++++++---
> hw/nvme/nvme.h | 1 +
> include/block/nvme.h | 36 ++++++++++++++++++
> 4 files changed, 111 insertions(+), 6 deletions(-)
>
> --
> 2.30.2
>
signature.asc
Description: PGP signature
- Re: [PATCH v4 0/2] Add OCP extended log to nvme QEMU,
Joel Granados <=