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