From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964958AbdCWRsV (ORCPT ); Thu, 23 Mar 2017 13:48:21 -0400 Received: from mail-by2nam03on0124.outbound.protection.outlook.com ([104.47.42.124]:56503 "EHLO NAM03-BY2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755143AbdCWRsR (ORCPT ); Thu, 23 Mar 2017 13:48:17 -0400 From: Long Li To: Vitaly Kuznetsov CC: KY Srinivasan , Haiyang Zhang , Stephen Hemminger , "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 Thread-Topic: [PATCH v2] HV: properly delay KVP packets when negotiation is in progress Thread-Index: AdKjPGiyFmKmSH4oS7qZJ2zu4OmE0QAsqHfLAAOYPPA= Date: Thu, 23 Mar 2017 17:47:13 +0000 Message-ID: References: <87bmssyo87.fsf@vitty.brq.redhat.com> In-Reply-To: <87bmssyo87.fsf@vitty.brq.redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [2001:4898:80e8:2::735] x-microsoft-exchange-diagnostics: 1;BN3PR03MB1414;7:1KLqOfp8kFOvDrYXV9gI55tfgxfE9J2yBybArkW1eXpxmBN6bjTJiT1g2JIz7G7ErcRsoNT1jsvLM9sarsm9gE3xZAnPjD8OFFEFW4dNFn/GhOrUwFZ8mFHdr6o/VbAx8z2xwHp1PJR//hFYxOZ1aIpGLnk4aSLnj2QKl5hrJ5woj7cULqmJBL9q7X1n9tWHn1tMUr0Qa/HUi29Ro/BksznzESSGKGZHlY6N0hGsU+C895Hc9rai6fLbMjmhgzNRWBjXfxIiV2mpfFjpxq9/NBmzOnU9KB4M8nFdFuoyQFQJ+qPO5Hadx82LGt9jpHuePY83t1Wg97Lv+Jcmrclbk8aIs0ARtiNJddlGvAmfZcc= x-ms-office365-filtering-correlation-id: 15b9eae6-d2cc-4fbb-b475-08d47214a3c2 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(2017030254075)(48565401081);SRVR:BN3PR03MB1414; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(9452136761055)(788757137089); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(61425038)(6040375)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026)(61426038)(61427038)(6041248)(20161123558025)(20161123562025)(20161123555025)(20161123564025)(20161123560025)(6042181)(6072148);SRVR:BN3PR03MB1414;BCL:0;PCL:0;RULEID:;SRVR:BN3PR03MB1414; x-forefront-prvs: 0255DF69B9 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(377454003)(13464003)(4326008)(122556002)(50986999)(76176999)(54356999)(38730400002)(7696004)(229853002)(6506006)(6246003)(110136004)(77096006)(6916009)(7736002)(99286003)(33656002)(305945005)(3660700001)(54906002)(3280700002)(55016002)(2950100002)(53546009)(551934003)(2906002)(74316002)(25786009)(53936002)(5005710100001)(6436002)(10290500002)(86612001)(10090500001)(6116002)(8676002)(2900100001)(189998001)(86362001)(9686003)(102836003)(8936002)(81166006)(5660300001);DIR:OUT;SFP:1102;SCL:1;SRVR:BN3PR03MB1414;H:BN3PR03MB2227.namprd03.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Mar 2017 17:47:13.4758 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR03MB1414 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2NHmZZw005806 > -----Original Message----- > From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com] > Sent: Thursday, March 23, 2017 9:04 AM > To: Long Li > Cc: KY Srinivasan ; Haiyang Zhang > ; Stephen Hemminger > ; 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 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 > > > > > > The patch v2 has incorporated suggestion from Vitaly Kuznetsov > > . > > > > Signed-off-by: Long Li > > --- > > 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