lkml.org 
[lkml]   [2017]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2] HV: properly delay KVP packets when negotiation is in progress
Date


> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Thursday, March 23, 2017 9:04 AM
> To: Long Li <longli@microsoft.com>
> Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2] HV: properly delay KVP packets when negotiation is
> in progress
>
> Long Li <longli@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.Meier@microsoft.com>
> >
> > The patch v2 has incorporated suggestion from Vitaly Kuznetsov
> > <vkuznets@redhat.com>.
> >
> > Signed-off-by: Long Li <longli@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

\
 
 \ /
  Last update: 2017-03-23 18:49    [W:0.555 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site