Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932071AbbDULfW (ORCPT ); Tue, 21 Apr 2015 07:35:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38362 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751482AbbDULfT (ORCPT ); Tue, 21 Apr 2015 07:35:19 -0400 From: Vitaly Kuznetsov To: Dexuan Cui Cc: KY Srinivasan , Haiyang Zhang , "devel\@linuxdriverproject.org" , "linux-kernel\@vger.kernel.org" Subject: Re: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths References: <1429604274-12537-1-git-send-email-vkuznets@redhat.com> <1429604274-12537-2-git-send-email-vkuznets@redhat.com> <8aa8661a8b9743aca9f54841c2622aa3@SIXPR30MB031.064d.mgd.msft.net> Date: Tue, 21 Apr 2015 13:35:08 +0200 In-Reply-To: <8aa8661a8b9743aca9f54841c2622aa3@SIXPR30MB031.064d.mgd.msft.net> (Dexuan Cui's message of "Tue, 21 Apr 2015 08:35:14 +0000") Message-ID: <87pp6xbtoz.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2303 Lines: 71 Dexuan Cui writes: >> -----Original Message----- >> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com] >> Sent: Tuesday, April 21, 2015 16:18 >> To: KY Srinivasan >> Cc: Haiyang Zhang; devel@linuxdriverproject.org; linux- >> kernel@vger.kernel.org; Dexuan Cui >> Subject: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() >> failure paths >> >> In case there was an error reported in the response to the >> CHANNELMSG_OPENCHANNEL >> call we need to do the cleanup as a vmbus_open() user won't be doing it >> after >> receiving an error. The cleanup should be done on all failure paths. >> >> Signed-off-by: Vitaly Kuznetsov >> --- >> drivers/hv/channel.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c >> index 54da66d..836386f 100644 >> --- a/drivers/hv/channel.c >> +++ b/drivers/hv/channel.c >> @@ -186,11 +186,12 @@ int vmbus_open(struct vmbus_channel >> *newchannel, u32 send_ringbuffer_size, >> list_del(&open_info->msglistentry); >> spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, >> flags); >> >> - if (err == 0) >> - newchannel->state = CHANNEL_OPENED_STATE; >> + if (err != 0) >> + goto error_gpadl; >> >> + newchannel->state = CHANNEL_OPENED_STATE; >> kfree(open_info); >> - return err; >> + return 0; >> >> error1: >> spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); >> -- > > Thanks for the patch, Vitaly! > > The patch looks good to me except for 1 small issue: > I suppose open_info->response.open_result.status is different from > Linux-style -EXXX error codes. > > Maybe here we can return -EAGAIN if err != 0 ? I think I inspected all vmbus_open() callers and they all just check ret != 0 but I agree we'd better return some -EXXXX code here. I'm not exactly sure if open_result.status != 0 usually means a permanent or a temporary failure but if it's temporary -EAGAIN here seems reasonable. I'll send v2, thanks for the review! > > -- Dexuan -- Vitaly -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/