"K. Y. Srinivasan" <kys@xxxxxxxxxxxxx> writes:
> Currently we have two policies for deciding when to signal the host:
> One based on the ring buffer state and the other based on what the
> VMBUS client driver wants to do. Consider the case when the client
> wants to explicitly control when to signal the host. In this case,
> if the client were to defer signaling, we will not be able to signal
> the host subsequently when the client does want to signal since the
> ring buffer state will prevent the signaling. Implement logic to
> have only one signaling policy in force for a given channel.
>
> Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> Reviewed-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Tested-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.2+
> ---
> drivers/hv/channel.c | 18 ++++++++++++++++++
> include/linux/hyperv.h | 12 ++++++++++++
> 2 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 77d2579..c6278c7 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -653,10 +653,19 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer,
> * on the ring. We will not signal if more data is
> * to be placed.
> *
> + * Based on the channel signal state, we will decide
> + * which signaling policy will be applied.
> + *
> * If we cannot write to the ring-buffer; signal the host
> * even if we may not have written anything. This is a rare
> * enough condition that it should not matter.
> */
> +
> + if (channel->signal_state)
> + signal = true;
> + else
> + kick_q = true;
> +
> if (((ret == 0) && kick_q && signal) || (ret))
> vmbus_setevent(channel);
>
> @@ -756,10 +765,19 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel,
> * on the ring. We will not signal if more data is
> * to be placed.
> *
> + * Based on the channel signal state, we will decide
> + * which signaling policy will be applied.
> + *
> * If we cannot write to the ring-buffer; signal the host
> * even if we may not have written anything. This is a rare
> * enough condition that it should not matter.
> */
> +
> + if (channel->signal_state)
> + signal = true;
> + else
> + kick_q = true;
> +
> if (((ret == 0) && kick_q && signal) || (ret))
> vmbus_setevent(channel);
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 437c9c8..7b1af52 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -756,8 +756,20 @@ struct vmbus_channel {
> * link up channels based on their CPU affinity.
> */
> struct list_head percpu_list;
> + /*
> + * Host signaling policy: The default policy will be
> + * based on the ring buffer state. We will also support
> + * a policy where the client driver can have explicit
> + * signaling control.
> + */
> + bool signal_state;
Making policy selector 'bool' is counter-intuitive: I suggest we rename
this to somethink like 'signal_explicit' if we're sure there won't be
new policies or (even better in my opinion) introduce a new enum with
these policies, e.g:
enum hv_channel_policy signal_policy;
where
enum hv_channel_policy {
HV_CHANNELPOLICY_DEFAULT,
HV_CHANNELPOLICY_EXPLICIT,
};
> };
>
> +static inline void set_channel_signal_state(struct vmbus_channel *c, bool state)
> +{
> + c->signal_state = state;
> +}
> +
> static inline void set_channel_read_state(struct vmbus_channel *c, bool state)
> {
> c->batched_reading = state;
--
Vitaly
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html