Re: [PATCH] replay: improve determinism of virtio-net

[Jason Wang <jasowang@redhat.com>]

在 2021/5/31 下午2:35, Pavel Dovgalyuk 写道:
> On 31.05.2021 07:55, Jason Wang wrote:
>>
>> 在 2021/5/17 下午9:04, Pavel Dovgalyuk 写道:
>>> virtio-net device uses bottom halves for callbacks.
>>> These callbacks should be deterministic, because they affect VM state.
>>> This patch replaces BH invocations with corresponding replay functions,
>>> making them deterministic in record/replay mode.
>>> This patch also disables guest announce timers for record/replay,
>>> because they break correct loadvm in deterministic mode.
>>
>>
>> Virtio-net can be configured to work in tx timer mode. Do we need to 
>> care about that?
>
> What does it mean? This patch fixes interaction with TX timer. Is it 
> related to that mode?


I meant is the timer used by virtio_net_handle_tx_timer() safe consider 
you disable announce timer.

Thanks


>
>>
>>
>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>>> ---
>>>   hw/net/virtio-net.c |   13 +++++++++----
>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 6b7e8dd04e..e876363236 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -44,6 +44,7 @@
>>>   #include "hw/pci/pci.h"
>>>   #include "net_rx_pkt.h"
>>>   #include "hw/virtio/vhost.h"
>>> +#include "sysemu/replay.h"
>>>   #define VIRTIO_NET_VM_VERSION    11
>>> @@ -394,7 +395,7 @@ static void virtio_net_set_status(struct 
>>> VirtIODevice *vdev, uint8_t status)
>>>                   timer_mod(q->tx_timer,
>>> qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);
>>>               } else {
>>> -                qemu_bh_schedule(q->tx_bh);
>>> +                replay_bh_schedule_event(q->tx_bh);
>>>               }
>>>           } else {
>>>               if (q->tx_timer) {
>>> @@ -2546,7 +2547,7 @@ static void 
>>> virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>>           return;
>>>       }
>>>       virtio_queue_set_notification(vq, 0);
>>> -    qemu_bh_schedule(q->tx_bh);
>>> +    replay_bh_schedule_event(q->tx_bh);
>>
>>
>> Not familiar with replay but any chance to change qemu_bh_schedule() 
>> instead?
>
> It would be better, but sometimes qemu_bh_schedule is used for the 
> callbacks that are not related to the guest state change.
>
>>
>> Thanks
>>
>>
>>>   }
>>>   static void virtio_net_tx_timer(void *opaque)
>>> @@ -2602,7 +2603,7 @@ static void virtio_net_tx_bh(void *opaque)
>>>       /* If we flush a full burst of packets, assume there are
>>>        * more coming and immediately reschedule */
>>>       if (ret >= n->tx_burst) {
>>> -        qemu_bh_schedule(q->tx_bh);
>>> +        replay_bh_schedule_event(q->tx_bh);
>>>           q->tx_waiting = 1;
>>>           return;
>>>       }
>>> @@ -2616,7 +2617,7 @@ static void virtio_net_tx_bh(void *opaque)
>>>           return;
>>>       } else if (ret > 0) {
>>>           virtio_queue_set_notification(q->tx_vq, 0);
>>> -        qemu_bh_schedule(q->tx_bh);
>>> +        replay_bh_schedule_event(q->tx_bh);
>>>           q->tx_waiting = 1;
>>>       }
>>>   }
>>> @@ -3206,6 +3207,10 @@ static void 
>>> virtio_net_device_realize(DeviceState *dev, Error **errp)
>>>           n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
>>>       }
>>> +    if (replay_mode != REPLAY_MODE_NONE) {
>>> +        n->host_features &= ~(1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE);
>>> +    }
>>> +
>>>       if (n->net_conf.duplex_str) {
>>>           if (strncmp(n->net_conf.duplex_str, "half", 5) == 0) {
>>>               n->net_conf.duplex = DUPLEX_HALF;
>>>
>>>
>>
>