[PATCH] nvme: honor the volatile write cache enable state

[Erwan Velu]

[...]
> Unfortunately, no: there are contradicting statements here.

Hi Keith,

Sorry for the confusion, my previous replies were wrong and contradicted
the commit message. Let me restate it with new measurements.

To make sure the numbers are relevant, I switched to the latest nvme
branch, nvme-7.4 (2e8e30b3cf9f), and re-ran all the tests from scratch,
with and without the patch on top of it, on the same drive (Kioxia
CD6-V, NVMe 1.4, VWC 0x5, WCE cleared by default).

Both rows of each job in the commit message table were measured with
WCE cleared; only the kernel differs. Without the patch, WCE is ignored
and the namespace is exposed in write-back mode, so one Flush is sent
per fsync. With the patch, the namespace is in write-through mode and no
Flush is sent.

To answer your question, I measured the Flush latency at the driver
level, between the nvme_setup_cmd and nvme_complete_rq tracepoints:

  - passthrough: 2000 "nvme flush" sent directly to the device
  - fio: 4k randwrite, io_uring, direct, QD1, fsync=1, 10s

   kernel  | WCE | mode    | passthrough | fio: Flush | fio: fsync |  IOPS
           |     |         | Flush avg   | avg (drv)  | avg (fio)  |
--------+-----+---------+-------------+------------+------------+------
   nvme-7.4|  0  | back    |    194.8us  |   214.6us  |  236.1us  |  3917
   patched |  0  | through |    194.2us  |    (none)  |  5.7us  | 44433
   patched |  1  | back    |    194.2us  |   219.7us  |  237.1us  |  3916

So:

  - The block layer flush handling is not the reason: without the patch,
    the device accounts for ~215us of the ~236us fsync, the rest of the
    stack for ~21us.

  - The drive spends ~194us on a Flush whether the volatile write cache
    is enabled or not. It does not short-circuit the Flush when WCE is
    cleared, although the spec says it has no effect in that case.

  - Without the patch, WCE cleared therefore performs exactly like the
    volatile write cache enabled (3917 vs 3916 IOPS), while the patched
    kernel skips the Flush entirely (44433 IOPS).

The slow no-op Flush is a firmware behavior, but the kernel should not
send it in the first place when WCE is cleared, which is what the patch
does.

I also re-ran the commit message benchmark on nvme-7.4, WCE cleared in
both cases (fio 4k randwrite, fsync=1, 30s):

   job                   | kernel   |   IOPS | fsync avg | flushes |  gain
----------------------+----------+--------+-----------+---------+------
   psync buffered, QD1   | nvme-7.4 |   3917 |   249.2us | 117506 |
                         | patched  |  48227 |    15.7us |  0 | x12.3
----------------------+----------+--------+-----------+---------+------
   io_uring direct, QD1  | nvme-7.4 |   3888 |   238.2us | 116636 |
                         | patched  |  42580 |     5.5us |  0 | x11.0
----------------------+----------+--------+-----------+---------+------
   io_uring direct, QD32 | nvme-7.4 |   5674 |  5514.1us | 117774 |
                         | patched  | 140090 |   107.1us |  0 | x24.7

If you think it makes the commit message clearer, I can send a v2 with
these numbers, the kernel anitly, and the driver level Flush latency above.

Thanks,
Erwan