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). 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. Thanks, Paolo