All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk
@ 2021-03-01 17:51 Enrico Granata
  2021-03-01 18:20 ` Stefan Hajnoczi
  2021-03-03 17:18 ` Cornelia Huck
  0 siblings, 2 replies; 7+ messages in thread
From: Enrico Granata @ 2021-03-01 17:51 UTC (permalink / raw)
  To: virtio-dev

In many embedded systems, virtio-blk implementations are
backed by eMMC or UFS storage devices, which are subject to
predictable and measurable wear over time due to repeated write
cycles.

For such systems, it can be important to be able to track
accurately the amount of wear imposed on the storage over
time and surface it to applications. In a native deployments
this is generally handled by the physical block device driver
but no such provision is made in virtio-blk to expose these
metrics for devices where it makes sense to do so.

This patch adds support to virtio-blk for lifetime and wear
metrics to be exposed to the guest when a deployment of
virtio-blk is done over compatible eMMC or UFS storage.

Signed-off-by: Enrico Granata <egranata@google.com>
---
 content.tex | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/content.tex b/content.tex
index 835f1ea..47e3566 100644
--- a/content.tex
+++ b/content.tex
@@ -4418,6 +4418,9 @@ \subsection{Feature bits}\label{sec:Device Types
/ Block Device / Feature bits}
 \item[VIRTIO_BLK_F_WRITE_ZEROES (14)] Device can support write zeroes command,
      maximum write zeroes sectors size in \field{max_write_zeroes_sectors} and
      maximum write zeroes segment number in \field{max_write_zeroes_seg}.
+
+\item[VIRTIO_BLK_F_LIFETIME (15)] Device supports providing storage lifetime
+     information.
 \end{description}

 \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
/ Block Device / Feature bits / Legacy Interface: Feature bits}
@@ -4601,14 +4604,16 @@ \subsection{Device Operation}\label{sec:Device
Types / Block Device / Device Ope

 The type of the request is either a read (VIRTIO_BLK_T_IN), a write
 (VIRTIO_BLK_T_OUT), a discard (VIRTIO_BLK_T_DISCARD), a write zeroes
-(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), or a get device ID
-string command (VIRTIO_BLK_T_GET_ID).
+(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), a get device ID
+string command (VIRTIO_BLK_T_GET_ID), or a get device lifetime
+command (VIRTIO_BLK_T_GET_LIFETIME).

 \begin{lstlisting}
 #define VIRTIO_BLK_T_IN           0
 #define VIRTIO_BLK_T_OUT          1
 #define VIRTIO_BLK_T_FLUSH        4
 #define VIRTIO_BLK_T_GET_ID       8
+#define VIRTIO_BLK_T_GET_LIFETIME 10
 #define VIRTIO_BLK_T_DISCARD      11
 #define VIRTIO_BLK_T_WRITE_ZEROES 13
 \end{lstlisting}
@@ -4648,6 +4653,23 @@ \subsection{Device Operation}\label{sec:Device
Types / Block Device / Device Ope
 \field{data}.  The device ID string is a NUL-padded ASCII string up to 20 bytes
 long.  If the string is 20 bytes long then there is no NUL terminator.

+The \field{data} used for VIRTIO_BLK_T_GET_LIFETIME requests is populated by
+the device, and is of the form:
+
+\begin{lstlisting}
+struct virtio_blk_lifetime {
+    le16 pre_eol_info;
+    le16 device_lifetime_est_a;
+    le16 device_lifetime_est_b;
+};
+\end{lstlisting}
+
+The device lifetime metrics \field{pre_eol_info}, \field{device_lifetime_est_a}
+and \field{device_lifetime_est_b} have the semantics described by the JEDEC
+standard No.84-B50 for the extended CSD register fields \field{PRE_EOL_INFO}
+\field{DEVICE_LIFETIME_EST_TYP_A} and \field{DEVICE_LIFETIME_EST_TYP_B}
+respectively.
+
 The final \field{status} byte is written by the device: either
 VIRTIO_BLK_S_OK for success, VIRTIO_BLK_S_IOERR for device or driver
 error or VIRTIO_BLK_S_UNSUPP for a request unsupported by device:
@@ -4754,6 +4776,11 @@ \subsection{Device Operation}\label{sec:Device
Types / Block Device / Device Ope
 (case~\ref{item:flush3}).  Failure to do so can cause data loss
 in case of a crash.

+If the device is backed by eMMC or UFS persistent storage, the device SHOULD
+offer the VIRTIO_BLK_F_LIFETIME flag. The flag MUST NOT be offered if
the device
+is backed by storage for which the lifetime metrics as described in
this document
+cannot be obtained or have no useful meaning.
+
 If the driver changes \field{writeback} between the submission of the write
 and its completion, the write could be either volatile or stable when
 its completion is reported; in other words, the exact behavior is undefined.
--
2.30.1.766.gb4fecdf3b7-goog

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk
  2021-03-01 17:51 [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk Enrico Granata
@ 2021-03-01 18:20 ` Stefan Hajnoczi
  2021-03-01 18:34   ` Enrico Granata
  2021-03-03 17:18 ` Cornelia Huck
  1 sibling, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-03-01 18:20 UTC (permalink / raw)
  To: Enrico Granata; +Cc: virtio-dev

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

On Mon, Mar 01, 2021 at 10:51:03AM -0700, Enrico Granata wrote:
> In many embedded systems, virtio-blk implementations are
> backed by eMMC or UFS storage devices, which are subject to
> predictable and measurable wear over time due to repeated write
> cycles.
> 
> For such systems, it can be important to be able to track
> accurately the amount of wear imposed on the storage over
> time and surface it to applications. In a native deployments
> this is generally handled by the physical block device driver
> but no such provision is made in virtio-blk to expose these
> metrics for devices where it makes sense to do so.
> 
> This patch adds support to virtio-blk for lifetime and wear
> metrics to be exposed to the guest when a deployment of
> virtio-blk is done over compatible eMMC or UFS storage.
> 
> Signed-off-by: Enrico Granata <egranata@google.com>
> ---
>  content.tex | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)

Great, please request a Technical Committee vote:
https://github.com/oasis-tcs/virtio-spec/#use-of-github-issues

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk
  2021-03-01 18:20 ` Stefan Hajnoczi
@ 2021-03-01 18:34   ` Enrico Granata
  0 siblings, 0 replies; 7+ messages in thread
From: Enrico Granata @ 2021-03-01 18:34 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: virtio-dev

TC leads, please schedule a vote on resolving this issue

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/97

Thanks,
- Enrico

On Mon, Mar 1, 2021 at 11:21 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Mar 01, 2021 at 10:51:03AM -0700, Enrico Granata wrote:
> > In many embedded systems, virtio-blk implementations are
> > backed by eMMC or UFS storage devices, which are subject to
> > predictable and measurable wear over time due to repeated write
> > cycles.
> >
> > For such systems, it can be important to be able to track
> > accurately the amount of wear imposed on the storage over
> > time and surface it to applications. In a native deployments
> > this is generally handled by the physical block device driver
> > but no such provision is made in virtio-blk to expose these
> > metrics for devices where it makes sense to do so.
> >
> > This patch adds support to virtio-blk for lifetime and wear
> > metrics to be exposed to the guest when a deployment of
> > virtio-blk is done over compatible eMMC or UFS storage.
> >
> > Signed-off-by: Enrico Granata <egranata@google.com>
> > ---
> >  content.tex | 31 +++++++++++++++++++++++++++++--
> >  1 file changed, 29 insertions(+), 2 deletions(-)
>
> Great, please request a Technical Committee vote:
> https://github.com/oasis-tcs/virtio-spec/#use-of-github-issues
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk
  2021-03-01 17:51 [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk Enrico Granata
  2021-03-01 18:20 ` Stefan Hajnoczi
@ 2021-03-03 17:18 ` Cornelia Huck
  2021-03-03 18:01   ` Enrico Granata
  1 sibling, 1 reply; 7+ messages in thread
From: Cornelia Huck @ 2021-03-03 17:18 UTC (permalink / raw)
  To: Enrico Granata; +Cc: virtio-dev

On Mon, 1 Mar 2021 10:51:03 -0700
Enrico Granata <egranata@google.com> wrote:

> In many embedded systems, virtio-blk implementations are
> backed by eMMC or UFS storage devices, which are subject to
> predictable and measurable wear over time due to repeated write
> cycles.
> 
> For such systems, it can be important to be able to track
> accurately the amount of wear imposed on the storage over
> time and surface it to applications. In a native deployments
> this is generally handled by the physical block device driver
> but no such provision is made in virtio-blk to expose these
> metrics for devices where it makes sense to do so.
> 
> This patch adds support to virtio-blk for lifetime and wear
> metrics to be exposed to the guest when a deployment of
> virtio-blk is done over compatible eMMC or UFS storage.
> 
> Signed-off-by: Enrico Granata <egranata@google.com>
> ---
>  content.tex | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index 835f1ea..47e3566 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -4418,6 +4418,9 @@ \subsection{Feature bits}\label{sec:Device Types
> / Block Device / Feature bits}

[something seems to have caused line wrapping in this patch]

>  \item[VIRTIO_BLK_F_WRITE_ZEROES (14)] Device can support write zeroes command,
>       maximum write zeroes sectors size in \field{max_write_zeroes_sectors} and
>       maximum write zeroes segment number in \field{max_write_zeroes_seg}.
> +
> +\item[VIRTIO_BLK_F_LIFETIME (15)] Device supports providing storage lifetime
> +     information.
>  \end{description}
> 
>  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
> / Block Device / Feature bits / Legacy Interface: Feature bits}
> @@ -4601,14 +4604,16 @@ \subsection{Device Operation}\label{sec:Device
> Types / Block Device / Device Ope
> 
>  The type of the request is either a read (VIRTIO_BLK_T_IN), a write
>  (VIRTIO_BLK_T_OUT), a discard (VIRTIO_BLK_T_DISCARD), a write zeroes
> -(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), or a get device ID
> -string command (VIRTIO_BLK_T_GET_ID).
> +(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), a get device ID
> +string command (VIRTIO_BLK_T_GET_ID), or a get device lifetime
> +command (VIRTIO_BLK_T_GET_LIFETIME).
> 
>  \begin{lstlisting}
>  #define VIRTIO_BLK_T_IN           0
>  #define VIRTIO_BLK_T_OUT          1
>  #define VIRTIO_BLK_T_FLUSH        4
>  #define VIRTIO_BLK_T_GET_ID       8
> +#define VIRTIO_BLK_T_GET_LIFETIME 10
>  #define VIRTIO_BLK_T_DISCARD      11
>  #define VIRTIO_BLK_T_WRITE_ZEROES 13
>  \end{lstlisting}
> @@ -4648,6 +4653,23 @@ \subsection{Device Operation}\label{sec:Device
> Types / Block Device / Device Ope
>  \field{data}.  The device ID string is a NUL-padded ASCII string up to 20 bytes
>  long.  If the string is 20 bytes long then there is no NUL terminator.
> 
> +The \field{data} used for VIRTIO_BLK_T_GET_LIFETIME requests is populated by
> +the device, and is of the form:
> +
> +\begin{lstlisting}
> +struct virtio_blk_lifetime {
> +    le16 pre_eol_info;
> +    le16 device_lifetime_est_a;
> +    le16 device_lifetime_est_b;
> +};
> +\end{lstlisting}
> +
> +The device lifetime metrics \field{pre_eol_info}, \field{device_lifetime_est_a}
> +and \field{device_lifetime_est_b} have the semantics described by the JEDEC
> +standard No.84-B50 for the extended CSD register fields \field{PRE_EOL_INFO}
> +\field{DEVICE_LIFETIME_EST_TYP_A} and \field{DEVICE_LIFETIME_EST_TYP_B}
> +respectively.

Do we have an explicit link to that JEDEC standard?

> +
>  The final \field{status} byte is written by the device: either
>  VIRTIO_BLK_S_OK for success, VIRTIO_BLK_S_IOERR for device or driver
>  error or VIRTIO_BLK_S_UNSUPP for a request unsupported by device:
> @@ -4754,6 +4776,11 @@ \subsection{Device Operation}\label{sec:Device
> Types / Block Device / Device Ope
>  (case~\ref{item:flush3}).  Failure to do so can cause data loss
>  in case of a crash.
> 
> +If the device is backed by eMMC or UFS persistent storage, the device SHOULD
> +offer the VIRTIO_BLK_F_LIFETIME flag. The flag MUST NOT be offered if
> the device
> +is backed by storage for which the lifetime metrics as described in
> this document
> +cannot be obtained or have no useful meaning.

Isn't that outside of the normative sections? If so, please make this a
description without SHOULD and MUST NOT, and add them to the normative
clauses.

Also, are eMMC/UFS just examples (i.e. may other types of persistent
storage provide these metrics as well?)

> +
>  If the driver changes \field{writeback} between the submission of the write
>  and its completion, the write could be either volatile or stable when
>  its completion is reported; in other words, the exact behavior is undefined.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk
  2021-03-03 17:18 ` Cornelia Huck
@ 2021-03-03 18:01   ` Enrico Granata
  2021-03-05 11:38     ` Cornelia Huck
  0 siblings, 1 reply; 7+ messages in thread
From: Enrico Granata @ 2021-03-03 18:01 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: virtio-dev

On Wed, Mar 3, 2021 at 10:18 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Mon, 1 Mar 2021 10:51:03 -0700
> Enrico Granata <egranata@google.com> wrote:
>
> > In many embedded systems, virtio-blk implementations are
> > backed by eMMC or UFS storage devices, which are subject to
> > predictable and measurable wear over time due to repeated write
> > cycles.
> >
> > For such systems, it can be important to be able to track
> > accurately the amount of wear imposed on the storage over
> > time and surface it to applications. In a native deployments
> > this is generally handled by the physical block device driver
> > but no such provision is made in virtio-blk to expose these
> > metrics for devices where it makes sense to do so.
> >
> > This patch adds support to virtio-blk for lifetime and wear
> > metrics to be exposed to the guest when a deployment of
> > virtio-blk is done over compatible eMMC or UFS storage.
> >
> > Signed-off-by: Enrico Granata <egranata@google.com>
> > ---
> >  content.tex | 31 +++++++++++++++++++++++++++++--
> >  1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/content.tex b/content.tex
> > index 835f1ea..47e3566 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -4418,6 +4418,9 @@ \subsection{Feature bits}\label{sec:Device Types
> > / Block Device / Feature bits}
>
> [something seems to have caused line wrapping in this patch]
>

Hmm... I can try applying again and sending out as a v4?

> >  \item[VIRTIO_BLK_F_WRITE_ZEROES (14)] Device can support write zeroes command,
> >       maximum write zeroes sectors size in \field{max_write_zeroes_sectors} and
> >       maximum write zeroes segment number in \field{max_write_zeroes_seg}.
> > +
> > +\item[VIRTIO_BLK_F_LIFETIME (15)] Device supports providing storage lifetime
> > +     information.
> >  \end{description}
> >
> >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
> > / Block Device / Feature bits / Legacy Interface: Feature bits}
> > @@ -4601,14 +4604,16 @@ \subsection{Device Operation}\label{sec:Device
> > Types / Block Device / Device Ope
> >
> >  The type of the request is either a read (VIRTIO_BLK_T_IN), a write
> >  (VIRTIO_BLK_T_OUT), a discard (VIRTIO_BLK_T_DISCARD), a write zeroes
> > -(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), or a get device ID
> > -string command (VIRTIO_BLK_T_GET_ID).
> > +(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), a get device ID
> > +string command (VIRTIO_BLK_T_GET_ID), or a get device lifetime
> > +command (VIRTIO_BLK_T_GET_LIFETIME).
> >
> >  \begin{lstlisting}
> >  #define VIRTIO_BLK_T_IN           0
> >  #define VIRTIO_BLK_T_OUT          1
> >  #define VIRTIO_BLK_T_FLUSH        4
> >  #define VIRTIO_BLK_T_GET_ID       8
> > +#define VIRTIO_BLK_T_GET_LIFETIME 10
> >  #define VIRTIO_BLK_T_DISCARD      11
> >  #define VIRTIO_BLK_T_WRITE_ZEROES 13
> >  \end{lstlisting}
> > @@ -4648,6 +4653,23 @@ \subsection{Device Operation}\label{sec:Device
> > Types / Block Device / Device Ope
> >  \field{data}.  The device ID string is a NUL-padded ASCII string up to 20 bytes
> >  long.  If the string is 20 bytes long then there is no NUL terminator.
> >
> > +The \field{data} used for VIRTIO_BLK_T_GET_LIFETIME requests is populated by
> > +the device, and is of the form:
> > +
> > +\begin{lstlisting}
> > +struct virtio_blk_lifetime {
> > +    le16 pre_eol_info;
> > +    le16 device_lifetime_est_a;
> > +    le16 device_lifetime_est_b;
> > +};
> > +\end{lstlisting}
> > +
> > +The device lifetime metrics \field{pre_eol_info}, \field{device_lifetime_est_a}
> > +and \field{device_lifetime_est_b} have the semantics described by the JEDEC
> > +standard No.84-B50 for the extended CSD register fields \field{PRE_EOL_INFO}
> > +\field{DEVICE_LIFETIME_EST_TYP_A} and \field{DEVICE_LIFETIME_EST_TYP_B}
> > +respectively.
>
> Do we have an explicit link to that JEDEC standard?
>

I believe the specification is under a JEDEC paywall. I have access to
it by means of my employer, but it doesn't seem to be publicly
available.

> > +
> >  The final \field{status} byte is written by the device: either
> >  VIRTIO_BLK_S_OK for success, VIRTIO_BLK_S_IOERR for device or driver
> >  error or VIRTIO_BLK_S_UNSUPP for a request unsupported by device:
> > @@ -4754,6 +4776,11 @@ \subsection{Device Operation}\label{sec:Device
> > Types / Block Device / Device Ope
> >  (case~\ref{item:flush3}).  Failure to do so can cause data loss
> >  in case of a crash.
> >
> > +If the device is backed by eMMC or UFS persistent storage, the device SHOULD
> > +offer the VIRTIO_BLK_F_LIFETIME flag. The flag MUST NOT be offered if
> > the device
> > +is backed by storage for which the lifetime metrics as described in
> > this document
> > +cannot be obtained or have no useful meaning.
>
> Isn't that outside of the normative sections? If so, please make this a
> description without SHOULD and MUST NOT, and add them to the normative
> clauses.
>

Hmm.. when I look at my local copy, I see this section right below the clause:

"If the device is backed by persistent storage, the device MUST ensure that
stable writes are committed to it, before reporting completion of the write
(cases~\ref{item:flush1} and~\ref{item:flush2}) or the flush
(case~\ref{item:flush3}).  Failure to do so can cause data loss
in case of a crash."

It looked like the right place to add this kind of clause to me, but
if I am wrong and you would like to see it moved, please do let me
know.
Maybe I really do just need to apply the patch again on a clean-slate
repository, if what I see locally does not align?

> Also, are eMMC/UFS just examples (i.e. may other types of persistent
> storage provide these metrics as well?)
>

I can't say I know of other hardware that provides identical metrics,
but it could be possible and maybe could be done by software
adaptation for other storage systems?

> > +
> >  If the driver changes \field{writeback} between the submission of the write
> >  and its completion, the write could be either volatile or stable when
> >  its completion is reported; in other words, the exact behavior is undefined.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk
  2021-03-03 18:01   ` Enrico Granata
@ 2021-03-05 11:38     ` Cornelia Huck
  2021-03-05 17:35       ` Enrico Granata
  0 siblings, 1 reply; 7+ messages in thread
From: Cornelia Huck @ 2021-03-05 11:38 UTC (permalink / raw)
  To: Enrico Granata; +Cc: virtio-dev

On Wed, 3 Mar 2021 11:01:50 -0700
Enrico Granata <egranata@google.com> wrote:

> On Wed, Mar 3, 2021 at 10:18 AM Cornelia Huck <cohuck@redhat.com> wrote:
> >
> > On Mon, 1 Mar 2021 10:51:03 -0700
> > Enrico Granata <egranata@google.com> wrote:
> >  
> > > In many embedded systems, virtio-blk implementations are
> > > backed by eMMC or UFS storage devices, which are subject to
> > > predictable and measurable wear over time due to repeated write
> > > cycles.
> > >
> > > For such systems, it can be important to be able to track
> > > accurately the amount of wear imposed on the storage over
> > > time and surface it to applications. In a native deployments
> > > this is generally handled by the physical block device driver
> > > but no such provision is made in virtio-blk to expose these
> > > metrics for devices where it makes sense to do so.
> > >
> > > This patch adds support to virtio-blk for lifetime and wear
> > > metrics to be exposed to the guest when a deployment of
> > > virtio-blk is done over compatible eMMC or UFS storage.
> > >
> > > Signed-off-by: Enrico Granata <egranata@google.com>
> > > ---
> > >  content.tex | 31 +++++++++++++++++++++++++++++--
> > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/content.tex b/content.tex
> > > index 835f1ea..47e3566 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -4418,6 +4418,9 @@ \subsection{Feature bits}\label{sec:Device Types
> > > / Block Device / Feature bits}  
> >
> > [something seems to have caused line wrapping in this patch]
> >  
> 
> Hmm... I can try applying again and sending out as a v4?

Please do, if we don't have any more changes on top.

> 
> > >  \item[VIRTIO_BLK_F_WRITE_ZEROES (14)] Device can support write zeroes command,
> > >       maximum write zeroes sectors size in \field{max_write_zeroes_sectors} and
> > >       maximum write zeroes segment number in \field{max_write_zeroes_seg}.
> > > +
> > > +\item[VIRTIO_BLK_F_LIFETIME (15)] Device supports providing storage lifetime
> > > +     information.
> > >  \end{description}
> > >
> > >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
> > > / Block Device / Feature bits / Legacy Interface: Feature bits}
> > > @@ -4601,14 +4604,16 @@ \subsection{Device Operation}\label{sec:Device
> > > Types / Block Device / Device Ope
> > >
> > >  The type of the request is either a read (VIRTIO_BLK_T_IN), a write
> > >  (VIRTIO_BLK_T_OUT), a discard (VIRTIO_BLK_T_DISCARD), a write zeroes
> > > -(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), or a get device ID
> > > -string command (VIRTIO_BLK_T_GET_ID).
> > > +(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), a get device ID
> > > +string command (VIRTIO_BLK_T_GET_ID), or a get device lifetime
> > > +command (VIRTIO_BLK_T_GET_LIFETIME).
> > >
> > >  \begin{lstlisting}
> > >  #define VIRTIO_BLK_T_IN           0
> > >  #define VIRTIO_BLK_T_OUT          1
> > >  #define VIRTIO_BLK_T_FLUSH        4
> > >  #define VIRTIO_BLK_T_GET_ID       8
> > > +#define VIRTIO_BLK_T_GET_LIFETIME 10
> > >  #define VIRTIO_BLK_T_DISCARD      11
> > >  #define VIRTIO_BLK_T_WRITE_ZEROES 13
> > >  \end{lstlisting}
> > > @@ -4648,6 +4653,23 @@ \subsection{Device Operation}\label{sec:Device
> > > Types / Block Device / Device Ope
> > >  \field{data}.  The device ID string is a NUL-padded ASCII string up to 20 bytes
> > >  long.  If the string is 20 bytes long then there is no NUL terminator.
> > >
> > > +The \field{data} used for VIRTIO_BLK_T_GET_LIFETIME requests is populated by
> > > +the device, and is of the form:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_blk_lifetime {
> > > +    le16 pre_eol_info;
> > > +    le16 device_lifetime_est_a;
> > > +    le16 device_lifetime_est_b;
> > > +};
> > > +\end{lstlisting}
> > > +
> > > +The device lifetime metrics \field{pre_eol_info}, \field{device_lifetime_est_a}
> > > +and \field{device_lifetime_est_b} have the semantics described by the JEDEC
> > > +standard No.84-B50 for the extended CSD register fields \field{PRE_EOL_INFO}
> > > +\field{DEVICE_LIFETIME_EST_TYP_A} and \field{DEVICE_LIFETIME_EST_TYP_B}
> > > +respectively.  
> >
> > Do we have an explicit link to that JEDEC standard?
> >  
> 
> I believe the specification is under a JEDEC paywall. I have access to
> it by means of my employer, but it doesn't seem to be publicly
> available.

That's unfortunate... how much of the information in there is needed
for someone to come up with a compliant implementation? Can we condense
that to a sentence or two that would be unproblematic to put into the
standard?

If JEDEC is providing reasonable licensing terms, I guess it could also
be an option to just refer to their homepage and specifying that
standard (I think the full name is JESD84-B50 -- at least that's what
my searching pointed to.)

> 
> > > +
> > >  The final \field{status} byte is written by the device: either
> > >  VIRTIO_BLK_S_OK for success, VIRTIO_BLK_S_IOERR for device or driver
> > >  error or VIRTIO_BLK_S_UNSUPP for a request unsupported by device:
> > > @@ -4754,6 +4776,11 @@ \subsection{Device Operation}\label{sec:Device
> > > Types / Block Device / Device Ope
> > >  (case~\ref{item:flush3}).  Failure to do so can cause data loss
> > >  in case of a crash.
> > >
> > > +If the device is backed by eMMC or UFS persistent storage, the device SHOULD
> > > +offer the VIRTIO_BLK_F_LIFETIME flag. The flag MUST NOT be offered if
> > > the device
> > > +is backed by storage for which the lifetime metrics as described in
> > > this document
> > > +cannot be obtained or have no useful meaning.  
> >
> > Isn't that outside of the normative sections? If so, please make this a
> > description without SHOULD and MUST NOT, and add them to the normative
> > clauses.
> >  
> 
> Hmm.. when I look at my local copy, I see this section right below the clause:
> 
> "If the device is backed by persistent storage, the device MUST ensure that
> stable writes are committed to it, before reporting completion of the write
> (cases~\ref{item:flush1} and~\ref{item:flush2}) or the flush
> (case~\ref{item:flush3}).  Failure to do so can cause data loss
> in case of a crash."
> 
> It looked like the right place to add this kind of clause to me, but
> if I am wrong and you would like to see it moved, please do let me
> know.

Ah, sorry, my mistake; I must have slipped in the document, your
addition is fine.

> Maybe I really do just need to apply the patch again on a clean-slate
> repository, if what I see locally does not align?
> 
> > Also, are eMMC/UFS just examples (i.e. may other types of persistent
> > storage provide these metrics as well?)
> >  
> 
> I can't say I know of other hardware that provides identical metrics,
> but it could be possible and maybe could be done by software
> adaptation for other storage systems?

If we can get potentially get the same metrics from other hardware,
that sentence should probably be reworded a bit. What about:

"If the device is backed by storage providing life time metrics (such
as eMMC or UFS persistent storage), the device SHOULD offer the
VIRTIO_BLK_F_LIFETIME flag."

?


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk
  2021-03-05 11:38     ` Cornelia Huck
@ 2021-03-05 17:35       ` Enrico Granata
  0 siblings, 0 replies; 7+ messages in thread
From: Enrico Granata @ 2021-03-05 17:35 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: virtio-dev

On Fri, Mar 5, 2021 at 4:38 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Wed, 3 Mar 2021 11:01:50 -0700
> Enrico Granata <egranata@google.com> wrote:
>
> > On Wed, Mar 3, 2021 at 10:18 AM Cornelia Huck <cohuck@redhat.com> wrote:
> > >
> > > On Mon, 1 Mar 2021 10:51:03 -0700
> > > Enrico Granata <egranata@google.com> wrote:
> > >
> > > > In many embedded systems, virtio-blk implementations are
> > > > backed by eMMC or UFS storage devices, which are subject to
> > > > predictable and measurable wear over time due to repeated write
> > > > cycles.
> > > >
> > > > For such systems, it can be important to be able to track
> > > > accurately the amount of wear imposed on the storage over
> > > > time and surface it to applications. In a native deployments
> > > > this is generally handled by the physical block device driver
> > > > but no such provision is made in virtio-blk to expose these
> > > > metrics for devices where it makes sense to do so.
> > > >
> > > > This patch adds support to virtio-blk for lifetime and wear
> > > > metrics to be exposed to the guest when a deployment of
> > > > virtio-blk is done over compatible eMMC or UFS storage.
> > > >
> > > > Signed-off-by: Enrico Granata <egranata@google.com>
> > > > ---
> > > >  content.tex | 31 +++++++++++++++++++++++++++++--
> > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/content.tex b/content.tex
> > > > index 835f1ea..47e3566 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -4418,6 +4418,9 @@ \subsection{Feature bits}\label{sec:Device Types
> > > > / Block Device / Feature bits}
> > >
> > > [something seems to have caused line wrapping in this patch]
> > >
> >
> > Hmm... I can try applying again and sending out as a v4?
>
> Please do, if we don't have any more changes on top.
>

Respun v4 at https://lists.oasis-open.org/archives/virtio-dev/202103/msg00038.html

> >
> > > >  \item[VIRTIO_BLK_F_WRITE_ZEROES (14)] Device can support write zeroes command,
> > > >       maximum write zeroes sectors size in \field{max_write_zeroes_sectors} and
> > > >       maximum write zeroes segment number in \field{max_write_zeroes_seg}.
> > > > +
> > > > +\item[VIRTIO_BLK_F_LIFETIME (15)] Device supports providing storage lifetime
> > > > +     information.
> > > >  \end{description}
> > > >
> > > >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
> > > > / Block Device / Feature bits / Legacy Interface: Feature bits}
> > > > @@ -4601,14 +4604,16 @@ \subsection{Device Operation}\label{sec:Device
> > > > Types / Block Device / Device Ope
> > > >
> > > >  The type of the request is either a read (VIRTIO_BLK_T_IN), a write
> > > >  (VIRTIO_BLK_T_OUT), a discard (VIRTIO_BLK_T_DISCARD), a write zeroes
> > > > -(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), or a get device ID
> > > > -string command (VIRTIO_BLK_T_GET_ID).
> > > > +(VIRTIO_BLK_T_WRITE_ZEROES), a flush (VIRTIO_BLK_T_FLUSH), a get device ID
> > > > +string command (VIRTIO_BLK_T_GET_ID), or a get device lifetime
> > > > +command (VIRTIO_BLK_T_GET_LIFETIME).
> > > >
> > > >  \begin{lstlisting}
> > > >  #define VIRTIO_BLK_T_IN           0
> > > >  #define VIRTIO_BLK_T_OUT          1
> > > >  #define VIRTIO_BLK_T_FLUSH        4
> > > >  #define VIRTIO_BLK_T_GET_ID       8
> > > > +#define VIRTIO_BLK_T_GET_LIFETIME 10
> > > >  #define VIRTIO_BLK_T_DISCARD      11
> > > >  #define VIRTIO_BLK_T_WRITE_ZEROES 13
> > > >  \end{lstlisting}
> > > > @@ -4648,6 +4653,23 @@ \subsection{Device Operation}\label{sec:Device
> > > > Types / Block Device / Device Ope
> > > >  \field{data}.  The device ID string is a NUL-padded ASCII string up to 20 bytes
> > > >  long.  If the string is 20 bytes long then there is no NUL terminator.
> > > >
> > > > +The \field{data} used for VIRTIO_BLK_T_GET_LIFETIME requests is populated by
> > > > +the device, and is of the form:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_blk_lifetime {
> > > > +    le16 pre_eol_info;
> > > > +    le16 device_lifetime_est_a;
> > > > +    le16 device_lifetime_est_b;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The device lifetime metrics \field{pre_eol_info}, \field{device_lifetime_est_a}
> > > > +and \field{device_lifetime_est_b} have the semantics described by the JEDEC
> > > > +standard No.84-B50 for the extended CSD register fields \field{PRE_EOL_INFO}
> > > > +\field{DEVICE_LIFETIME_EST_TYP_A} and \field{DEVICE_LIFETIME_EST_TYP_B}
> > > > +respectively.
> > >
> > > Do we have an explicit link to that JEDEC standard?
> > >
> >
> > I believe the specification is under a JEDEC paywall. I have access to
> > it by means of my employer, but it doesn't seem to be publicly
> > available.
>
> That's unfortunate... how much of the information in there is needed
> for someone to come up with a compliant implementation? Can we condense
> that to a sentence or two that would be unproblematic to put into the
> standard?
>
> If JEDEC is providing reasonable licensing terms, I guess it could also
> be an option to just refer to their homepage and specifying that
> standard (I think the full name is JESD84-B50 -- at least that's what
> my searching pointed to.)
>

I have added an explicit mention of finding the spec at JEDEC website.
If you think it is worthwhile to try and condense the spec in here, I
can take a pass at it.
I do slightly prefer just pointing the interested reader at JEDEC for
a couple reasons:
- in case of ambiguity, the JEDEC spec would be normative  and the
content here just "FYI" at best, so the interested implementor would
be best advised to consult the actual spec anyway;
- in a lot of common cases, the hardware will provide this data via
the host system's interface (e.g. sysfs in Linux) and need no further
processing by the virtio-blki implementation.

Up to you though. I can work with either just the link, or adding a
brief description

> >
> > > > +
> > > >  The final \field{status} byte is written by the device: either
> > > >  VIRTIO_BLK_S_OK for success, VIRTIO_BLK_S_IOERR for device or driver
> > > >  error or VIRTIO_BLK_S_UNSUPP for a request unsupported by device:
> > > > @@ -4754,6 +4776,11 @@ \subsection{Device Operation}\label{sec:Device
> > > > Types / Block Device / Device Ope
> > > >  (case~\ref{item:flush3}).  Failure to do so can cause data loss
> > > >  in case of a crash.
> > > >
> > > > +If the device is backed by eMMC or UFS persistent storage, the device SHOULD
> > > > +offer the VIRTIO_BLK_F_LIFETIME flag. The flag MUST NOT be offered if
> > > > the device
> > > > +is backed by storage for which the lifetime metrics as described in
> > > > this document
> > > > +cannot be obtained or have no useful meaning.
> > >
> > > Isn't that outside of the normative sections? If so, please make this a
> > > description without SHOULD and MUST NOT, and add them to the normative
> > > clauses.
> > >
> >
> > Hmm.. when I look at my local copy, I see this section right below the clause:
> >
> > "If the device is backed by persistent storage, the device MUST ensure that
> > stable writes are committed to it, before reporting completion of the write
> > (cases~\ref{item:flush1} and~\ref{item:flush2}) or the flush
> > (case~\ref{item:flush3}).  Failure to do so can cause data loss
> > in case of a crash."
> >
> > It looked like the right place to add this kind of clause to me, but
> > if I am wrong and you would like to see it moved, please do let me
> > know.
>
> Ah, sorry, my mistake; I must have slipped in the document, your
> addition is fine.
>
> > Maybe I really do just need to apply the patch again on a clean-slate
> > repository, if what I see locally does not align?
> >
> > > Also, are eMMC/UFS just examples (i.e. may other types of persistent
> > > storage provide these metrics as well?)
> > >
> >
> > I can't say I know of other hardware that provides identical metrics,
> > but it could be possible and maybe could be done by software
> > adaptation for other storage systems?
>
> If we can get potentially get the same metrics from other hardware,
> that sentence should probably be reworded a bit. What about:
>
> "If the device is backed by storage providing life time metrics (such
> as eMMC or UFS persistent storage), the device SHOULD offer the
> VIRTIO_BLK_F_LIFETIME flag."
>

Reworded as you suggest in v4. Thanks for the feedback

> ?
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-05 17:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 17:51 [virtio-dev] [PATCH v3] Add lifetime metrics to virtio-blk Enrico Granata
2021-03-01 18:20 ` Stefan Hajnoczi
2021-03-01 18:34   ` Enrico Granata
2021-03-03 17:18 ` Cornelia Huck
2021-03-03 18:01   ` Enrico Granata
2021-03-05 11:38     ` Cornelia Huck
2021-03-05 17:35       ` Enrico Granata

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.