From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933454AbcI2JWN (ORCPT ); Thu, 29 Sep 2016 05:22:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49948 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932185AbcI2JWG (ORCPT ); Thu, 29 Sep 2016 05:22:06 -0400 From: Vitaly Kuznetsov To: "K. Y. Srinivasan" , Long Li Cc: Haiyang Zhang , devel@linuxdriverproject.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] hv: do not lose pending heartbeat vmbus packets References: <1473737481-21939-1-git-send-email-longli@exchange.microsoft.com> Date: Thu, 29 Sep 2016 11:22:02 +0200 In-Reply-To: <1473737481-21939-1-git-send-email-longli@exchange.microsoft.com> (Long Li's message of "Mon, 12 Sep 2016 20:31:21 -0700") Message-ID: <87lgyb3vt1.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 29 Sep 2016 09:22:05 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Long Li writes: > From: Long Li > > The host keeps sending heartbeat packets independent of guest responding to them. In some situations, there might be multiple heartbeat packets pending in the ring buffer. Don't lose them, read them all. > > Signed-off-by: Long Li Long, K. Y., it seems this patch didn't make it to char-misc tree and it looks like an important fix. A couple of nitpicks below, > --- > drivers/hv/hv_util.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c > index d5acaa2..9dc6372 100644 > --- a/drivers/hv/hv_util.c > +++ b/drivers/hv/hv_util.c > @@ -283,10 +283,14 @@ static void heartbeat_onchannelcallback(void *context) > u8 *hbeat_txf_buf = util_heartbeat.recv_buffer; > struct icmsg_negotiate *negop = NULL; > > - vmbus_recvpacket(channel, hbeat_txf_buf, > - PAGE_SIZE, &recvlen, &requestid); > + while (1) { > + > + vmbus_recvpacket(channel, hbeat_txf_buf, > + PAGE_SIZE, &recvlen, &requestid); We should check vmbus_recvpacket() return value as well. E.g. hv_ringbuffer_read() may return -EAGAIN in case we didn't receive the whole packet (and we do this check in other drivers, see storvsc_on_channel_callback() for example). > + > + if (!recvlen) so this should be 'if (ret || !recvlen)' > + break; > > - if (recvlen > 0) { > icmsghdrp = (struct icmsg_hdr *)&hbeat_txf_buf[ > sizeof(struct vmbuspipe_hdr)]; -- Vitaly