I've been doing some looking at local storage I/O latency and one nice improvement will be to only have the emulated NVMe controller report a volatile write cache if that reflects the semantics of the backing medium. this is correct for Crucible storage, as Crucible fast-acks the writes, and is probably necessary for file-backed disks backed by .. normal files.
for file-backed disks using raw zvols (as in the case of local storage in the product), we use /dev/zvol/rdsk/ paths which are not buffered. at that point, if the underlying storage device does not have a write cache, we the guest's NVMe disk doesn't need to report one either. the OS only completes the write once the underlying disk has completed the write, and Propolis only completes the write once the OS has completed the write, so when the guest sees the write complete then the device which promises the write is persistent has done the job.
I've done some fishing around in guest OSes and on the Linux side the OS short-circuits flushes directed towards block devices "without write caches" at blk_insert_flush in blk-flush.c, with a morally similar short-circuiting happening in illumos at nvme_bd_sync in common/io/nvme/nvme.c. some quick checking by simply clearing the vwc bit and rebuilding propolis-standalone show that workloads heavy on sync() are immediately ~3x faster (slightly more than I expected!), so without vwc guests do clearly do less work.
I don't think Propolis should try to decide if vwc is correct or not given a particular instance spec. for development (like my workstation, CI, etc) I probably shouldn't need a physical device without vwc to test how guests behave in such cases. more importantly in the product we may want to still report a write cache when hardware "doesn't have one". this is morally similar to how we may want to hide CPUID features for platform compatibility and do so by decision in Nexus, though not migrating local storage simplifies things somewhat.
(I'm already putting together a change for this one, it's fairly tiny all-told)
I've been doing some looking at local storage I/O latency and one nice improvement will be to only have the emulated NVMe controller report a volatile write cache if that reflects the semantics of the backing medium. this is correct for Crucible storage, as Crucible fast-acks the writes, and is probably necessary for file-backed disks backed by .. normal files.
for file-backed disks using raw zvols (as in the case of local storage in the product), we use
/dev/zvol/rdsk/paths which are not buffered. at that point, if the underlying storage device does not have a write cache, we the guest's NVMe disk doesn't need to report one either. the OS only completes the write once the underlying disk has completed the write, and Propolis only completes the write once the OS has completed the write, so when the guest sees the write complete then the device which promises the write is persistent has done the job.I've done some fishing around in guest OSes and on the Linux side the OS short-circuits flushes directed towards block devices "without write caches" at
blk_insert_flushinblk-flush.c, with a morally similar short-circuiting happening in illumos atnvme_bd_syncincommon/io/nvme/nvme.c. some quick checking by simply clearing thevwcbit and rebuilding propolis-standalone show that workloads heavy on sync() are immediately ~3x faster (slightly more than I expected!), so withoutvwcguests do clearly do less work.I don't think Propolis should try to decide if vwc is correct or not given a particular instance spec. for development (like my workstation, CI, etc) I probably shouldn't need a physical device without vwc to test how guests behave in such cases. more importantly in the product we may want to still report a write cache when hardware "doesn't have one". this is morally similar to how we may want to hide CPUID features for platform compatibility and do so by decision in Nexus, though not migrating local storage simplifies things somewhat.
(I'm already putting together a change for this one, it's fairly tiny all-told)