Re: [RFC 0/1] hw/nvme: add atomic write support

[Klaus Jensen]

On Apr 15 16:46, Alan Adamson wrote:
> Since there is discussion in the Linux NVMe Driver community to add NVMe 
> Atomic Write
> support, it would be desirable to test it with qemu nvme emulation.
>  
> Initially, this RFC will focus on supporting NVMe controller atomic write 
> parameters(AWUN,
> AWUPF, and ACWU) but will be extended to support Namespace parameters (NAWUN, 
> NAWUPF
> and NACWU).
>  
> Atomic Write Parameters for NVMe QEMU
> -------------------------------------
> New NVMe QEMU Parameters (See NVMe Specification for details):
>         atomic.dn (default off) - Set the value of Disable Normal.
>         atomic.awun=UINT16 (default: 0)
>         atomic.awupf=UINT16 (default: 0)
>         atomic.acwu=UINT16 (default: 0)
>  
> qemu command line example:
>         qemu-system-x86_64 -cpu host --enable-kvm -smp cpus=4 -no-reboot -m 
> 8192M -drive file=./disk.img,if=ide \
>         -boot c -device e1000,netdev=net0,mac=DE:CC:CC:EF:99:88 -netdev 
> tap,id=net0 \
>         -device 
> nvme,id=nvme-ctrl-0,serial=nvme-1,atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0
>  \
>         -drive file=./nvme.img,if=none,id=nvm-1 -device 
> nvme-ns,drive=nvm-1,bus=nvme-ctrl-0 nvme-ns,drive=nvm-1,bus=nvme-ctrl-0
>  
> Making Writes Atomic:
> ---------------------
> - Prior to a command being pulled off the SQ and executed, a check is made to 
> see if it
>   conflicts "atomically" with a currently executing command.
> - All currently executing commands on the same namespace, across all SQs need 
> to be checked.
> - If an atomic conflict is detected, the command is not started and remains 
> on the queue.
>  
> Testing
> -------
> NVMe QEMU Parameters used: 
> atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0
>  
> # nvme id-ctrl /dev/nvme0 | grep awun
> awun      : 63
> # nvme id-ctrl /dev/nvme0 | grep awupf
> awupf     : 63
> # nvme id-ctrl /dev/nvme0 | grep acwu
> acwu      : 0    < Since qemu-nvme doesn't support Compare and Write, this is 
> always zero
> # nvme get-feature /dev/nvme0  -f 0xa
> get-feature:0x0a (Write Atomicity Normal), Current value:00000000
> #
>  
> # fio --filename=/dev/nvme0n1 --direct=1 --rw=randwrite --bs=32k 
> --iodepth=256 --name=iops --numjobs=50 --verify=crc64 --verify_fatal=1 
> --ioengine=libaio
>  
> When executed without atomic write support, eventually the following error 
> will be
> observed:
>  
>         crc64: verify failed at file /dev/nvme0n1 offset 857669632, length 
> 32768
> (requested block: offset=857669632, length=32768, flags=88)
>             Expected CRC: 9c87d3539dafdca0
>             Received CRC: d521f7ea3b69d2ee
>  
> When executed with atomic write support, this error no longer happens.
>  
> Questions
> ---------
> AWUN vs AWUPF - Does the nvme emulation need to do treat these differently? 
> Currently the
> larger of the two will be used as the max atomic write size.
>  
> Future Work
> -----------
> - Namespace support (NAWUN, NAWUPF and NACWU)
> - Namespace Boundary support (NABSN, NABO, and NABSPF)
> - Atomic Compare and Write Unit (ACWU)
> 
> Alan Adamson (1):
>   nvme: add atomic write support
> 
>  hw/nvme/ctrl.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/nvme/nvme.h |  17 ++++++
>  2 files changed, 163 insertions(+), 1 deletion(-)
> 
> -- 
> 2.39.3
> 

Hi Alan,

I have no obvious qualms about this. It is clearly useful for driver
testing and verification and does not negatively impact the performance
when this "faked" feature is not enabled.

Acked-by: Klaus Jensen <k.jensen@samsung.com>

signature.asc
Description: PGP signature