The WCE issue in guest when i enable WCE in Qemu side.

Commit Message

Yang Zhong July 23, 2019, 12:17 p.m. UTC

Hello all,

I am using the Qemu/SPDK to do vhost-user-blk verification, and the qemu command as below:

./qemu-system-x86_64 \
    -machine q35,accel=kvm \
    -cpu host -m 1024,maxmem=20G,slots=2 -smp 6 \
    -kernel  ./../linux-stable/arch/x86/boot/bzImage \
    -append 'root=/dev/vdb3 rw rootfstype=ext4 data=ordered rcupdate.rcu_expedited=1 pci=lastbus=0 tsc=reliable no_timer_check reboot=t noapictimer console=hvc0' \
    -no-user-config -nodefaults \
    -drive file=/home/chen/project/clear-18080-kvm.img,if=virtio,format=raw -nographic -monitor pty \
    -device virtio-serial-pci,id=virtio-serial0,disable-modern \
    -chardev stdio,id=charconsole0 \
    -device virtconsole,chardev=charconsole0,id=console \
    -chardev socket,id=char0,path=/var/tmp/vhost.1 \
    -device vhost-user-blk-pci,chardev=char0,num-queues=1,bootindex=2,config-wce=true \
    -object memory-backend-file,id=mem1,size=1G,mem-path=/dev/hugepages,share=on \
    -numa node,memdev=mem1

When I set config-wce=true or false, the below value never change
 root@unicorn ~ # cat /sys/block/vda/cache_type
write back
root@unicorn ~ # cat /sys/block/vda/device/features
0010 0010 0110 0110 0000 0000 0000 1000 1000 0000 0000 0000 0000 0000 0000 0000 
From left to right, bit 11=0, which show there is no WCE feature in vhost-user-blk device.

I also did one simple patch to add config_wce value into s->blkcfg.wce, but the result is same.

Thanks a lot!

Regards,

Yang

Comments

Paolo Bonzini July 23, 2019, 3:32 p.m. UTC | #1

On 23/07/19 14:17, Zhong, Yang wrote:
> When I set config-wce=true or false, the below value never change
>  root@unicorn ~ # cat /sys/block/vda/cache_type
> write back
> root@unicorn ~ # cat /sys/block/vda/device/features
> 0010 0010 0110 0110 0000 0000 0000 1000 1000 0000 0000 0000 0000 0000 0000 0000 
> From left to right, bit 11=0, which show there is no WCE feature in vhost-user-blk device.

Does your backend expose the feature in the VHOST_GET_FEATURES message?

> I also did one simple patch to add config_wce value into s->blkcfg.wce, but the result is same.

config_wce doesn't tell you if you are in writeback or writethrough
mode, it tells you if you can *change* the mode.

If your backend did not expose the feature, try changing that and then
"echo write through > /sys/block/vda/cache_type" should work?

Paolo

Yang Zhong July 24, 2019, 6:55 a.m. UTC | #2

On Tue, Jul 23, 2019 at 05:32:12PM +0200, Paolo Bonzini wrote:
> On 23/07/19 14:17, Zhong, Yang wrote:
> > When I set config-wce=true or false, the below value never change
> >  root@unicorn ~ # cat /sys/block/vda/cache_type
> > write back
> > root@unicorn ~ # cat /sys/block/vda/device/features
> > 0010 0010 0110 0110 0000 0000 0000 1000 1000 0000 0000 0000 0000 0000 0000 0000 
> > From left to right, bit 11=0, which show there is no WCE feature in vhost-user-blk device.
> 
> Does your backend expose the feature in the VHOST_GET_FEATURES message?

  Paolo, thanks for your comments.
  
  The get features value in vhost-user-blk is 0x154007646(bit 11 is 0), which
  does not include WCE feature in default of SPDK vhost-blk module.

> > I also did one simple patch to add config_wce value into s->blkcfg.wce, but the result is same.
> 
> config_wce doesn't tell you if you are in writeback or writethrough
> mode, it tells you if you can *change* the mode.
> 
> If your backend did not expose the feature, try changing that and then
> "echo write through > /sys/block/vda/cache_type" should work?
> 
  Paolo, yes, i enabled WCE feature in the backend code(SPDK), and did
  the test config-wce= true or false as below

  (1).config-wce=true in qemu command for vhosr-user-blk's perporty 
      ##root@unicorn ~ # cat /sys/block/vda/device/features
       0010 0010 0111 0110 000000000000100010000000000000000000000000000000 ---> bit 11 is "1", which support WCE feature.

     ##set the "write through"
       echo write through > /sys/block/vda/cache_type
       root@unicorn ~ # cat /sys/block/vda/cache_type
       write through

   (2).config-wce=false in qemu command for vhost-ser-blk
      root@unicorn ~ # cat /sys/block/vda/cache_type
      write back
      root@unicorn ~ # cat /sys/block/vda/device/features
      0010 0010 0110 0110 0000 0000 0000 1000 1000 0000 0000 00000000000000000000 ---> bit 11 is "0" because of config-wce=false
      root@unicorn ~ # echo "write through" > /sys/block/vda/cache_type
      -bash: /sys/block/vda/cache_type: Permission denied

      Only config-wce=true can change the cache mode, thanks.

      The SPDK code i am using is latest version, So, i feel we should enable WCE feature in SPDK side by default. thanks for your help!
       
      Yang
> Paolo

Paolo Bonzini July 24, 2019, 8:57 a.m. UTC | #3

On 24/07/19 08:55, Yang Zhong wrote:
> 
> The SPDK code i am using is latest version, So, i feel we should
> enable WCE feature in SPDK side by default. thanks for your help!

Note that enabling the CONFIG_WCE feature has some consequences.
According to the spec:

- if the VIRTIO_BLK_F_CONFIG_WCE feature was negotiated and the
writeback field in configuration space was 0 all the time between the
submission of the write and its completion, the device MUST ensure that
stable writes are committed to persistent storage before reporting
completion of the write.  This means using FUA writes or something like that

- the device MUST offset VIRTIO_BLK_F_FLUSH if they offer
VIRTIO_BLK_F_CONFIG_WCE

- if VIRTIO_BLK_F_CONFIG_WCE is negotiated but VIRTIO_BLK_F_FLUSH is
not, the device MUST initialize writeback to 0

Paolo

Yang Zhong July 24, 2019, 12:47 p.m. UTC | #4

On Wed, Jul 24, 2019 at 10:57:30AM +0200, Paolo Bonzini wrote:
> On 24/07/19 08:55, Yang Zhong wrote:
> > 
> > The SPDK code i am using is latest version, So, i feel we should
> > enable WCE feature in SPDK side by default. thanks for your help!
> 
> Note that enabling the CONFIG_WCE feature has some consequences.
> According to the spec:
> 
> - if the VIRTIO_BLK_F_CONFIG_WCE feature was negotiated and the
> writeback field in configuration space was 0 all the time between the
> submission of the write and its completion, the device MUST ensure that
> stable writes are committed to persistent storage before reporting
> completion of the write.  This means using FUA writes or something like that
> 
> - the device MUST offset VIRTIO_BLK_F_FLUSH if they offer
> VIRTIO_BLK_F_CONFIG_WCE
> 
> - if VIRTIO_BLK_F_CONFIG_WCE is negotiated but VIRTIO_BLK_F_FLUSH is
> not, the device MUST initialize writeback to 0
>
  Paolo, thanks for your reminder, these info are very helpful to me:),
  thanks, Yang.

> Paolo

Patch

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9cb61336a6..0e3f490c82 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -447,6 +447,9 @@  reconnect:
         s->blkcfg.num_queues = s->num_queues;
     }

+    if (s->blkcfg.wce != s->config_wce) {
+        s->blkcfg.wce = s->config_wce;
+    }
     return;

It seems the config-wce property setting in Qemu is not working, and I am not sure if this is a bug or vhost-user-blk only support write back mode ?