data race in virtio net about gokvm

[bobuhiro11]

A simple lock/unlock as shown below resolves the data race but fails the unit tests.
I wonder why.

Click to expand

$ git --no-pager diff
diff --git a/virtio/blk.go b/virtio/blk.go
index 4dfeaf10f3f9..4f57874598e4 100644
--- a/virtio/blk.go
+++ b/virtio/blk.go
@@ -4,6 +4,7 @@ import (
        "bytes"
        "encoding/binary"
        "os"
+       "sync"
        "unsafe"

        "github.com/bobuhiro11/gokvm/pci"
@@ -28,6 +29,7 @@ type Blk struct {

        irq         uint8
        IRQInjector IRQInjector
+       mux         sync.Mutex
 }

 type blkHdr struct {
@@ -49,7 +51,10 @@ type blkHeader struct {
        capacity uint64
 }

-func (v Blk) GetDeviceHeader() pci.DeviceHeader {
+func (v *Blk) GetDeviceHeader() pci.DeviceHeader {
+       v.mux.Lock()
+       defer v.mux.Unlock()
+
        return pci.DeviceHeader{
                DeviceID:    0x1001,
                VendorID:    0x1AF4,
@@ -66,7 +71,10 @@ func (v Blk) GetDeviceHeader() pci.DeviceHeader {
        }
 }

-func (v Blk) IOInHandler(port uint64, bytes []byte) error {
+func (v *Blk) IOInHandler(port uint64, bytes []byte) error {
+       v.mux.Lock()
+       defer v.mux.Unlock()
+
        offset := int(port - BlkIOPortStart)

        b, err := v.Hdr.Bytes()
@@ -147,10 +155,16 @@ func (v *Blk) IO() error {
                }

                usedRing.Idx++
+
+               v.mux.Lock()
                v.LastAvailIdx[sel]++
+               v.mux.Unlock()
        }

+       v.mux.Lock()
        v.Hdr.commonHeader.isr = 0x1
+       v.mux.Unlock()
+
        if err := v.IRQInjector.InjectVirtioBlkIRQ(); err != nil {
                return err
        }
@@ -159,6 +173,9 @@ func (v *Blk) IO() error {
 }

 func (v *Blk) IOOutHandler(port uint64, bytes []byte) error {
+       v.mux.Lock()
+       defer v.mux.Unlock()
+
        offset := int(port - BlkIOPortStart)

        switch offset {
diff --git a/virtio/net.go b/virtio/net.go
index 7c2c18a2e130..01cec8286e40 100644
--- a/virtio/net.go
+++ b/virtio/net.go
@@ -8,6 +8,7 @@ import (
        "io"
        "os"
        "os/signal"
+       "sync"
        "syscall"
        "unsafe"

@@ -47,6 +48,7 @@ type Net struct {

        irq         uint8
        IRQInjector IRQInjector
+       mux         sync.Mutex
 }

 func (h netHdr) Bytes() ([]byte, error) {
@@ -65,7 +67,10 @@ type netHeader struct {
        _ uint16   // maxVirtQueuePairs
 }

-func (v Net) GetDeviceHeader() pci.DeviceHeader {
+func (v *Net) GetDeviceHeader() pci.DeviceHeader {
+       v.mux.Lock()
+       defer v.mux.Unlock()
+
        return pci.DeviceHeader{
                DeviceID:    0x1000,
                VendorID:    0x1AF4,
@@ -82,15 +87,20 @@ func (v Net) GetDeviceHeader() pci.DeviceHeader {
        }
 }

-func (v Net) IOInHandler(port uint64, bytes []byte) error {
+func (v *Net) IOInHandler(port uint64, bytes []byte) error {
+       v.mux.Lock()
+       defer v.mux.Unlock()
+
        offset := int(port - NetIOPortStart)

        b, err := v.Hdr.Bytes()
+
        if err != nil {
                return err
        }

        l := len(bytes)
+
        copy(bytes[:l], b[offset:offset+l])

        return nil
@@ -119,7 +129,10 @@ func (v *Net) Rx() error {

        sel := 0

-       if v.VirtQueue[sel] == nil {
+       v.mux.Lock()
+       q :=  v.VirtQueue[sel]
+       v.mux.Unlock()
+       if q == nil {
                return ErrVQNotInit
        }

@@ -171,7 +184,9 @@ func (v *Net) Rx() error {

        usedRing.Idx++

+       v.mux.Lock()
        v.Hdr.commonHeader.isr = 0x1
+       v.mux.Unlock()

        return v.IRQInjector.InjectVirtioNetIRQ()
 }
@@ -229,15 +244,23 @@ func (v *Net) Tx() error {
                        return err
                }
                usedRing.Idx++
+
+               v.mux.Lock()
                v.LastAvailIdx[sel]++
+               v.mux.Unlock()
        }

+       v.mux.Lock()
        v.Hdr.commonHeader.isr = 0x1
+       v.mux.Unlock()

        return v.IRQInjector.InjectVirtioNetIRQ()
 }

 func (v *Net) IOOutHandler(port uint64, bytes []byte) error {
+       v.mux.Lock()
+       defer v.mux.Unlock()
+
        offset := int(port - NetIOPortStart)

        switch offset {