> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Thursday, March 23, 2017 9:04 AM
> To: Long Li <lon...@microsoft.com>
> Cc: KY Srinivasan <k...@microsoft.com>; Haiyang Zhang
> <haiya...@microsoft.com>; Stephen Hemminger
> <sthem...@microsoft.com>; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org; sta...@vger.kernel.org
> Subject: Re: [PATCH v2] HV: properly delay KVP packets when negotiation is
> in progress
>
> Long Li <lon...@microsoft.com> writes:
>
> > The host may send multiple negotiation packets (due to timeout) before
> > the KVP user-mode daemon is connected. We need to defer processing
> > those packets until the daemon is negotiated and connected. It's okay
> > for guest to respond to all negotiation packets.
> >
> > In addition, the host may send multiple staged KVP requests as soon as
> > negotiation is done. We need to properly process those packets using
> > one tasklet for exclusive access to ring buffer.
> >
> > This patch is based on the work of Nick Meier
> > <nick.me...@microsoft.com>
> >
> > The patch v2 has incorporated suggestion from Vitaly Kuznetsov
> > <vkuzn...@redhat.com>.
> >
> > Signed-off-by: Long Li <lon...@microsoft.com>
> > ---
> > drivers/hv/hv_kvp.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index
> > de26371..845b70b 100644
> > --- a/drivers/hv/hv_kvp.c
> > +++ b/drivers/hv/hv_kvp.c
> > @@ -113,7 +113,7 @@ static void kvp_poll_wrapper(void *channel) {
> > /* Transaction is finished, reset the state here to avoid races. */
> > kvp_transaction.state = HVUTIL_READY;
> > - hv_kvp_onchannelcallback(channel);
> > + tasklet_schedule(&((struct vmbus_channel*)channel)-
> >callback_event);
> > }
>
> There is one more function in the code which calls
> hv_kvp_onchannelcallback():
>
> static void kvp_host_handshake_func(struct work_struct *dummy) {
> hv_poll_channel(kvp_transaction.recv_channel,
> hv_kvp_onchannelcallback); }
>
> we can't replace hv_kvp_onchannelcallback with kvp_poll_wrapper here as
> we don't want to reset kvp_transaction.state but it seems this should also
> get updated, e.g. hv_poll_channel() here can be replaced with the direct
>
> tasklet_schedule(&((struct vmbus_channel*)channel)->callback_event);
>
> call. This will ensure hv_kvp_onchannelcallback() calls are always serialized.
Thank you. I will send v3.
>
> >
> > static void kvp_register_done(void)
> > @@ -628,16 +628,17 @@ void hv_kvp_onchannelcallback(void *context)
> > NEGO_IN_PROGRESS,
> > NEGO_FINISHED} host_negotiatied =
> NEGO_NOT_STARTED;
> >
> > - if (host_negotiatied == NEGO_NOT_STARTED &&
> > - kvp_transaction.state < HVUTIL_READY) {
> > + if (kvp_transaction.state < HVUTIL_READY) {
> > /*
> > * If userspace daemon is not connected and host is asking
> > * us to negotiate we need to delay to not lose messages.
> > * This is important for Failover IP setting.
> > */
> > - host_negotiatied = NEGO_IN_PROGRESS;
> > - schedule_delayed_work(&kvp_host_handshake_work,
> > + if (host_negotiatied == NEGO_NOT_STARTED) {
> > + host_negotiatied = NEGO_IN_PROGRESS;
> > +
> schedule_delayed_work(&kvp_host_handshake_work,
> > HV_UTIL_NEGO_TIMEOUT * HZ);
> > + }
> > return;
> > }
> > if (kvp_transaction.state > HVUTIL_READY) @@ -705,6 +706,7 @@
> void
> > hv_kvp_onchannelcallback(void *context)
> > VM_PKT_DATA_INBAND, 0);
> >
> > host_negotiatied = NEGO_FINISHED;
> > + hv_poll_channel(kvp_transaction.recv_channel,
> kvp_poll_wrapper);
> > }
> >
> > }
>
> --
> Vitaly
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
