hyperv: vmbus: ring buffer optimization on host pending content to write

[honzhan_microsoft.com]

Summary

Once the VSP was blocked on writing ring buffer if there is no space, VSP
maintains the pending content, and informs VSC the length of pending data.
After reading data, VSC has to check the space on ring buffer, if it is
big enough for writing, it will inform VSP.

Submitted by: Hongjiang Zhang <honzhan microsoft com>

[howard0su_gmail.com]

sys/dev/hyperv/include/hyperv.h

680

define this as a constant? like #define PENDING_SEND_SZ 0x00001?

sys/dev/hyperv/vmbus/hv_channel.c

815

not needed.

815

bsd kernel use bool instead of boolean_t. although both are defined.

sys/dev/hyperv/vmbus/hv_ring_buffer.c

395

style(9), also mark as inline

[honzhan_microsoft.com]

According to howard0su_gmail.com's suggestion, I did corresponding modifications except for "boolean_t==>bool" in order to keep consistent in current file. In the future, I will do a special code refactor in order to align with FreeBSD's mainstream style.

In addition, I fixed another small potential issue: the return value in hv_ring_buffer_read is not used.

I did not observed apparant performance impact with this feature when I run performance test through iperf3.

sys/dev/hyperv/include/hyperv.h

680

Agree. Good suggestion.

[honzhan_microsoft.com]

replace "false" with "FALSE" in order to be consistent with other "FALSE" in the same file.

[decui_microsoft.com]

LGTM.

[howard0su_gmail.com]

sys/dev/hyperv/include/hyperv.h

683

I mean, define constant and feature_bits should be a uint32.

[honzhan_microsoft.com]

sys/dev/hyperv/include/hyperv.h

683

You want to define like below, right?

uint32 feature_bits;

IMHO, the current implementation looks to be redundant, but it is consistent with current implementation style. It delivers information of
(1) feature bits contains 32 bits, only used 1 bit currently
(2) you can easily access each bit through a field name

[sepherosa_gmail.com]

sys/dev/hyperv/include/hyperv.h

680

Please avoid using bit fields; just define it as uint32_t and use OR, AND etc. to operate on it.

sys/dev/hyperv/vmbus/hv_ring_buffer.c

491

Is it correct to do this testing out of the ring spin lock?

[honzhan_microsoft.com]

sys/dev/hyperv/vmbus/hv_ring_buffer.c

491

Tesing out of ring spin lock is on purpose. The check in hv_need_to_signal_on_read is:
“if ((prev_write_sz < pending_sz) && (cur_write_sz >= pending_sz))”
It is triggered only when there is another ring_read happens before "hv_need_to_signal_on_read" and after "mtx_unlock_spin".

[sepherosa_gmail.com]

No longer applies; and I don't think we would need it.