From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753128AbdBMTS2 (ORCPT ); Mon, 13 Feb 2017 14:18:28 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:26003 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752504AbdBMTS1 (ORCPT ); Mon, 13 Feb 2017 14:18:27 -0500 Subject: Re: [PATCH v4 net-next 5/8] sunvnet: straighten up message event handling logic To: Joe Perches , netdev@vger.kernel.org, davem@davemloft.net References: <1487012224-403399-1-git-send-email-shannon.nelson@oracle.com> <1487012224-403399-6-git-send-email-shannon.nelson@oracle.com> <1487012802.6214.3.camel@perches.com> Cc: sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org From: Shannon Nelson Organization: Oracle Corporation Message-ID: <073f1737-3990-1a69-fc31-ed86c8fd64bd@oracle.com> Date: Mon, 13 Feb 2017 11:18:12 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <1487012802.6214.3.camel@perches.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/13/2017 11:06 AM, Joe Perches wrote: > On Mon, 2017-02-13 at 10:57 -0800, Shannon Nelson wrote: >> The use of gotos for handling the incoming events made this code >> harder to read and support than it should be. This patch straightens >> out and clears up the logic. >> >> Signed-off-by: Shannon Nelson >> --- >> drivers/net/ethernet/sun/sunvnet_common.c | 94 ++++++++++++++--------------- >> 1 files changed, 45 insertions(+), 49 deletions(-) >> >> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c > [] >> @@ -738,41 +738,37 @@ static int vnet_event_napi(struct vnet_port *port, int budget) > [] >> + /* we don't expect any other bits */ >> + BUG_ON(port->rx_event & ~(LDC_EVENT_DATA_READY | >> + LDC_EVENT_RESET | >> + LDC_EVENT_UP)); > > Is it really necessary to use BUG_ON here? > I'm carrying this from the original code because we want to know asap if we have a low level protocol issue. It should never happen in the field, but we want to notice it as soon as we can when doing development and testing. In this patch I've simply made it more obvious and up front that we're doing this test rather than having it buried in the logic a few lines further down. sln From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shannon Nelson Date: Mon, 13 Feb 2017 19:18:12 +0000 Subject: Re: [PATCH v4 net-next 5/8] sunvnet: straighten up message event handling logic Message-Id: <073f1737-3990-1a69-fc31-ed86c8fd64bd@oracle.com> List-Id: References: <1487012224-403399-1-git-send-email-shannon.nelson@oracle.com> <1487012224-403399-6-git-send-email-shannon.nelson@oracle.com> <1487012802.6214.3.camel@perches.com> In-Reply-To: <1487012802.6214.3.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Joe Perches , netdev@vger.kernel.org, davem@davemloft.net Cc: sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org On 2/13/2017 11:06 AM, Joe Perches wrote: > On Mon, 2017-02-13 at 10:57 -0800, Shannon Nelson wrote: >> The use of gotos for handling the incoming events made this code >> harder to read and support than it should be. This patch straightens >> out and clears up the logic. >> >> Signed-off-by: Shannon Nelson >> --- >> drivers/net/ethernet/sun/sunvnet_common.c | 94 ++++++++++++++--------------- >> 1 files changed, 45 insertions(+), 49 deletions(-) >> >> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c > [] >> @@ -738,41 +738,37 @@ static int vnet_event_napi(struct vnet_port *port, int budget) > [] >> + /* we don't expect any other bits */ >> + BUG_ON(port->rx_event & ~(LDC_EVENT_DATA_READY | >> + LDC_EVENT_RESET | >> + LDC_EVENT_UP)); > > Is it really necessary to use BUG_ON here? > I'm carrying this from the original code because we want to know asap if we have a low level protocol issue. It should never happen in the field, but we want to notice it as soon as we can when doing development and testing. In this patch I've simply made it more obvious and up front that we're doing this test rather than having it buried in the logic a few lines further down. sln