[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 11/34] hyperv: add synic message delivery
From: |
Roman Kagan |
Subject: |
Re: [Qemu-devel] [RFC PATCH 11/34] hyperv: add synic message delivery |
Date: |
Wed, 7 Feb 2018 22:06:15 +0300 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Wed, Feb 07, 2018 at 11:58:08AM +0100, Paolo Bonzini wrote:
> On 06/02/2018 21:30, Roman Kagan wrote:
> > +
> > + HvSintMsgCb msg_cb;
> > + void *msg_cb_data;
> > + struct hyperv_message *msg;
> > + /*
> > + * the state of the message staged in .msg:
> > + * 0 - the staging area is not in use (after init or message
> > + * successfully delivered to guest)
> > + * -EBUSY - the staging area is being used in vcpu thread
> > + * -EAGAIN - delivery attempt failed due to slot being busy, retry
> > + * -EXXXX - error
> > + */
> > + int msg_status;
> > +
>
> Access to these fields needs to be protected by a mutex (the refcount is
> okay because it is only released in the bottom half).
Hmm, I'll double-check, but the original idea was that this stuff is
only used/ref-d/unref-d in the main thread so no mutex was necessary.
> Please also add
> comments regarding which fields are read-only, which are accessed under
> BQL, etc.
>
> Also, msg_status's handling of EBUSY/EAGAIN is a bit strange, like:
>
> + if (ret == -EBUSY) {
> + return -EAGAIN;
> + }
> + if (ret) {
> + return ret;
> + }
>
> I wonder if it would be better to change -EBUSY to 1, or to split
> msg_status into two fields, such as msg_status (filled by the vCPU
> thread) and msg_state which can be HYPERV_MSG_STATE_{FREE,BUSY,POSTED}.
> msg_status is only valid if the state is POSTED, and sint_msg_bh then
> moves the state back to FREE.
Fair enough, that code isn't easy to follow. I'll give this your
suggestion a try.
Thanks,
Roman.
[Qemu-devel] [RFC PATCH 12/34] hyperv: add synic event flag signaling, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 13/34] hyperv: process SIGNAL_EVENT hypercall, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 14/34] hyperv: process POST_MESSAGE hypercall, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 16/34] hyperv: update copyright notices, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 15/34] hyperv_testdev: add SynIC message and event testmodes, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 17/34] [not to commit] import HYPERV_EVENTFD stuff from kernel, Roman Kagan, 2018/02/06