VIRTIO_BLK_F_CONFIG_WCE is important in order to achieve good performance (up to 2x, though more realistically +30-40%) in latency-bound workloads. However, it was removed by mistake together with VIRTIO_BLK_F_FLUSH. In addition, even removing VIRTIO_BLK_F_FLUSH was probably not a great idea, because it simplifies simple drivers (e.g. firmware) that are okay with a writethrough cache but still need data to persist after power loss. What really should have been removed is just the possibility that devices not propose VIRTIO_BLK_F_FLUSH, but even that only deserves a "SHOULD" in the new world of conformance statements. Restore these, with the following changes: * clarify and use conformance statements in order to define writeback and writethrough caching according to what is commonly done by high-end storage. * clarify (with conformance statements) the influence of the VIRTIO_BLK_F_FLUSH feature on caching and how to proceed if only one of VIRTIO_BLK_F_FLUSH and VIRTIO_BLK_F_CONFIG_WCE is negotiated. * strengthen the requirement for persisting writes to MUST after a VIRTIO_BLK_T_FLUSH request (and in other cases too involving the new features). The suggested behavior upon feature negotiation is okay for the Linux implementation of virtio1, even after the implementation is modified to support the two new features. This fixes VIRTIO-144. Signed-off-by: Paolo Bonzini <
pbonzini@redhat.com> --- v3->v4: - do not mention FEATURES_OK wrt initialization of field{writeback} - s/non-volatile storage/permanent device backend storage/ - s/propose/offer/ - use MUST/SHOULD/MAY in the legacy interface sections - clarify undefined behavior - clarify workaround for lack of writethrough emulation in older legacy devices Remaining issues: - do we need to explicitly allow non-volatile storage (tmpfs, cache=unsafe, etc.)? - is it RECOMMENDED ("SHOULD") or OPTIONAL ("MAY") that devices keep data safe when they do not offer VIRTIO_BLK_F_FLUSH? Note that the driver cannot expect anything even if we say "SHOULD". conformance.tex 2 + content.tex 136 +++++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 117 insertions(+), 21 deletions(-) diff --git a/conformance.tex b/conformance.tex index 29c6ba8..8753cd6 100644 --- a/conformance.tex +++ b/conformance.tex @@ -102,6 +102,7 @@ A network driver MUST conform to the following normative statements: A block driver MUST conform to the following normative statements: egin{itemize} +item
ef{drivernormative:Device Types / Block Device / Device Initialization} item
ef{drivernormative:Device Types / Block Device / Device Operation} end{itemize} @@ -205,6 +206,7 @@ A network device MUST conform to the following normative statements: A block device MUST conform to the following normative statements: egin{itemize} +item
ef{devicenormative:Device Types / Block Device / Device Initialization} item
ef{devicenormative:Device Types / Block Device / Device Operation} end{itemize} diff --git a/content.tex b/content.tex index 3b12263..55a0381 100644 --- a/content.tex +++ b/content.tex @@ -3723,8 +3723,13 @@ device except where noted. item[VIRTIO_BLK_F_BLK_SIZE (6)] Block size of disk is in field{blk_size}. +item[VIRTIO_BLK_F_FLUSH (9)] Cache flush command support. + item[VIRTIO_BLK_F_TOPOLOGY (10)] Device exports information on optimal I/O alignment. + +item[VIRTIO_BLK_F_CONFIG_WCE (11)] Device can toggle its cache between writeback + and writethrough modes. end{description} subsubsection{Legacy Interface: Feature bits}label{sec:Device Types / Block Device / Feature bits / Legacy Interface: Feature bits} @@ -3733,16 +3738,12 @@ device except where noted. item[VIRTIO_BLK_F_BARRIER (0)] Device supports request barriers. item[VIRTIO_BLK_F_SCSI (7)] Device supports scsi packet commands. - -item[VIRTIO_BLK_F_FLUSH (9)] Cache flush command support. - -item[VIRTIO_BLK_F_CONFIG_WCE (11)] Device can toggle its cache between writeback - and writethrough modes. end{description} -VIRTIO_BLK_F_FLUSH was also called VIRTIO_BLK_F_WCE: Legacy drivers -MUST only negotiate this feature if they are capable of sending -VIRTIO_BLK_T_FLUSH commands. +egin{note} + In the legacy interface, VIRTIO_BLK_F_FLUSH was also + called VIRTIO_BLK_F_WCE. +end{note} subsection{Device configuration layout}label{sec:Device Types / Block Device / Device configuration layout} @@ -3771,7 +3772,7 @@ struct virtio_blk_config { // optimal (suggested maximum) I/O size in blocks le32 opt_io_size; } topology; - u8 reserved; + u8 writeback; }; end{lstlisting} @@ -3801,21 +3802,56 @@ according to the native endian of the guest rather than field{topology} struct can be read to determine the physical block size and optimal I/O lengths for the driver to use. This also does not affect the units in the protocol, only performance. + +item If the VIRTIO_BLK_F_CONFIG_WCE feature is negotiated, the cache + mode can be read or set through the field{writeback} field. 0 corresponds + to a writethrough cache, 1 to a writeback cachefootnote{Consistent with +
ef{devicenormative:Device Types / Block Device / Device Operation}, + a writethrough cache can be defined broadly as a cache that commits + writes to permanent device backend storage before reporting their + completion. For example, a battery-backed writeback cache actually + counts as writethrough according to this definition.}. The cache mode + after reset is undefined. end{enumerate} +drivernormative{subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization} + +Drivers SHOULD NOT negotiate VIRTIO_BLK_F_FLUSH if they are incapable of +sending VIRTIO_BLK_T_FLUSH commands. + +If the VIRTIO_BLK_F_CONFIG_WCE feature is not negotiated, but +VIRTIO_BLK_F_FLUSH was offered by the device, the driver MAY deduce +the presence of a writethrough cache. Otherwise, the driver SHOULD +assume presence of a writeback cache. + +devicenormative{subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization} + +Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it +if they offer VIRTIO_BLK_F_CONFIG_WCE. + +If VIRTIO_BLK_F_CONFIG_WCE is negotiated but the VIRTIO_BLK_F_FLUSH feature +is not, the device MUST initialize field{writeback} to 0. + subsubsection{Legacy Interface: Device Initialization}label{sec:Device Types / Block Device / Device Initialization / Legacy Interface: Device Initialization} -The field{reserved} field used to be called field{writeback}. If the -VIRTIO_BLK_F_CONFIG_WCE feature is offered, the cache mode can be -read from field{writeback}; the -driver can also write to the field in order to toggle the cache -between writethrough (0) and writeback (1) mode. If the feature is -not available, the driver can instead look at the result of -negotiating VIRTIO_BLK_F_FLUSH: the cache will be in writeback mode -after reset if and only if VIRTIO_BLK_F_FLUSH is negotiated. +Because legacy devices do not have FEATURES_OK, transitional devices +MUST implement slightly different behavior around feature negotiation +when used through the legacy interface. In particular: + +egin{itemize} +item they MUST NOT modify field{writeback} as a result + of a driver's write to the status register. + Some legacy drivers wrote to field{writeback} between + setting the DRIVER status bit and setting the DRIVER_OK status bit. + Transitional device MUST NOT overwrite field{writeback} when + the DRIVER_OK status bit is set. + +item they MUST NOT modify field{writeback} as a result of + a driver's write to the guest features registers, for example if the + driver sets the VIRTIO_BLK_F_CONFIG_WCE guest feature bit but does not + set the VIRTIO_BLK_F_FLUSH bit. +end{itemize} -Some older legacy devices did not operate in writethrough mode even -after a driver announced lack of support for VIRTIO_BLK_F_FLUSH. subsection{Device Operation}label{sec:Device Types / Block Device / Device Operation} @@ -3865,14 +3901,66 @@ A driver SHOULD accept the VIRTIO_BLK_F_RO feature if offered. A driver MUST set field{sector} to 0 for a VIRTIO_BLK_T_FLUSH request. A driver SHOULD NOT include any data in a VIRTIO_BLK_T_FLUSH request. +If the VIRTIO_BLK_F_CONFIG_WCE feature is negotiated, the driver MAY +switch to writethrough or writeback mode by writing respectively 0 and +1 to the field{writeback} field. After writing a 0 to field{writeback}, +the driver MUST NOT assume that any volatile writes have been committed +to permanent device backend storage. + devicenormative{subsubsection}{Device Operation}{Device Types / Block Device / Device Operation} A device MUST set the field{status} byte to VIRTIO_BLK_S_IOERR for a write request if the VIRTIO_BLK_F_RO feature if offered, and MUST NOT write any data. -Upon receipt of a VIRTIO_BLK_T_FLUSH request, the device SHOULD ensure -that any writes which were completed are committed to non-volatile storage. +A write is considered volatile when it is submitted; the contents of +sectors covered by a volatile are undefined until the write becomes stable. +A write becomes stable once it is completed and one or more of the +following conditions is true: + +egin{enumerate} +itemlabel{item:flush1} neither VIRTIO_BLK_F_CONFIG_WCE nor + VIRTIO_BLK_F_FLUSH feature were negotiated, but VIRTIO_BLK_F_FLUSH was + offered by the device; + +itemlabel{item:flush2} the VIRTIO_BLK_F_CONFIG_WCE feature was negotiated and the + field{writeback} field in configuration space was 0 extbf{all the time between + the submission of the write and its completion}; + +itemlabel{item:flush3} a VIRTIO_BLK_T_FLUSH request is sent extbf{after the write is + completed} and is completed itself. +end{enumerate} + +The device MUST ensure that stable writes are committed to +permanent device backend storage, before reporting completion of the write +(cases~
ef{item:flush1} and~
ef{item:flush2}) or the flush +(case~
ef{item:flush3}). Failure to do so can cause data loss +in case of a crash. + +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. + +% According to the device requirements for device initialization: +% Propose(CONFIG_WCE) => Propose(FLUSH). +% +% After reversing the implication: +% not Propose(FLUSH) => not Propose(CONFIG_WCE). + +If VIRTIO_BLK_F_FLUSH was not offered by the + devicefootnote{Note that in this case, according to +
ef{devicenormative:Device Types / Block Device / Device Initialization}, + the device will not have offered VIRTIO_BLK_F_CONFIG_WCE either.}, the +device MAY also commit writes to permanent device backend storage before +reporting their completion. Unlike case~
ef{item:flush1}, however, this +is not an absolute requirement of the specification. + +egin{note} + An implementation that does not offer VIRTIO_BLK_F_FLUSH and does not commit + completed writes will not be resilient to data loss in case of crashes. + Not offering VIRTIO_BLK_F_FLUSH is an absolute requirement + for implementations that do not wish to be safe against such data losses. +end{note} subsubsection{Legacy Interface: Device Operation}label{sec:Device Types / Block Device / Device Operation / Legacy Interface: Device Operation} When using the legacy interface, transitional devices and drivers @@ -3907,6 +3995,12 @@ serve as data consistency guarantee. Only a VIRTIO_BLK_T_FLUSH request does that. end{note} +Some older legacy devices did not commit completed writes to permanent +device backend storage when VIRTIO_BLK_F_FLUSH was offered but not +negotiated. In order to work around this, the driver MAY set the +field{writeback} to 0 (if available) or it MAY send an explicit flush +request after every completed write. + If the device has VIRTIO_BLK_F_SCSI feature, it can also support scsi packet command requests, each of these requests is of form: -- 2.4.3