All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: pbonzini@redhat.com, Jason Wang <jasowang@redhat.com>,
	Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH] replay: improve determinism of virtio-net
Date: Wed, 20 Oct 2021 10:50:17 -0400	[thread overview]
Message-ID: <20211020104507-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <8735ovx0zd.fsf@linaro.org>

On Wed, Oct 20, 2021 at 02:40:18PM +0100, Alex Bennée wrote:
> 
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
> 
> > On 31.05.2021 07:55, Jason Wang wrote:
> >> 在 2021/5/17 下午9:04, Pavel Dovgalyuk 写道:
> <snip>
> >>> @@ -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.
> 
> Maybe that indicates we should expand the API:
> 
>   void qemu_bh_schedule(QEMUBH *bh, bool guest_state);
> 
> or maybe explicit functions:
> 
>   void qemu_bh_schedule(QEMUBH *bh);
>   void qemu_bh_schedule_guest_update(QEMUBH *bh);
> 
> And document the cases with proper prototypes in main-loop.h.
> 
> While I was poking around I also saw qemu_bh_schedule_idle which made me
> wonder what happens if a guest change is triggered by this. Would this
> be impossible to make deterministic because we don't know when the bh
> actually get activated?
> 
> My concern with just adding replay_bh_schedule_event in the appropriate
> places is we end up with a patchwork of support for different devices
> and make it very easy to break things.

Right. In fact I think the default should be guest update,
and some kind of new API to be used for things that are
not guest visible.

We really need static analysis to enforce this kind of
constraint, too.


> -- 
> Alex Bennée



  reply	other threads:[~2021-10-20 14:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-17 13:04 [PATCH] replay: improve determinism of virtio-net Pavel Dovgalyuk
2021-05-31  4:46 ` Pavel Dovgalyuk
2021-05-31  4:55 ` Jason Wang
2021-05-31  6:35   ` Pavel Dovgalyuk
2021-05-31  6:39     ` Jason Wang
2021-05-31  8:47       ` Pavel Dovgalyuk
2021-06-01 10:33       ` Pavel Dovgalyuk
2021-10-20 13:40     ` Alex Bennée
2021-10-20 14:50       ` Michael S. Tsirkin [this message]
2021-07-02 15:11 ` Michael S. Tsirkin
2021-07-02 15:22 ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211020104507-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=jasowang@redhat.com \
    --cc=pavel.dovgalyuk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.