From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF691C4360C for ; Fri, 4 Oct 2019 18:18:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C7D1A215EA for ; Fri, 4 Oct 2019 18:18:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="52FMSBoG" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728962AbfJDSSe (ORCPT ); Fri, 4 Oct 2019 14:18:34 -0400 Received: from vps0.lunn.ch ([185.16.172.187]:33198 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725775AbfJDSSe (ORCPT ); Fri, 4 Oct 2019 14:18:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=KgvhD2KWl5QxHzscY/loy1FsMMZ1HUlegulTjj4OyJE=; b=52FMSBoGGOKPLgNs3i/kbh/7FL eXbUyHUHxfa+uhxWpnFAnNmgIkMdYavEoPyi4t47cNMSLnnMluHzr/hoE+H0V8RDJQrQ/fTnGqeot shjdKR8VdNOjmbrk/ZXthPGEmDhZnAT7yFngWxrSHtluHeyDUrU4WWngdUWrXXGP4X90=; Received: from andrew by vps0.lunn.ch with local (Exim 4.92.2) (envelope-from ) id 1iGS9s-0002jd-Ix; Fri, 04 Oct 2019 20:18:28 +0200 Date: Fri, 4 Oct 2019 20:18:28 +0200 From: Andrew Lunn To: Ioana Ciornei Cc: davem@davemloft.net, netdev@vger.kernel.org, Ioana Radulescu Subject: Re: [PATCH 3/3] dpaa2-eth: Avoid unbounded while loops Message-ID: <20191004181828.GB9935@lunn.ch> References: <1570180893-9538-1-git-send-email-ioana.ciornei@nxp.com> <1570180893-9538-4-git-send-email-ioana.ciornei@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1570180893-9538-4-git-send-email-ioana.ciornei@nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Oct 04, 2019 at 12:21:33PM +0300, Ioana Ciornei wrote: > From: Ioana Radulescu > > Throughout the driver there are several places where we wait > indefinitely for DPIO portal commands to be executed, while > the portal returns a busy response code. > > Even though in theory we are guaranteed the portals become > available eventually, in practice the QBMan hardware module > may become unresponsive in various corner cases. > > Make sure we can never get stuck in an infinite while loop > by adding a retry counter for all portal commands. > > Signed-off-by: Ioana Radulescu > Signed-off-by: Ioana Ciornei > --- > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 30 ++++++++++++++++++++---- > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h | 8 +++++++ > 2 files changed, 33 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > index 2c5072fa9aa0..29702756734c 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > @@ -221,6 +221,7 @@ static void xdp_release_buf(struct dpaa2_eth_priv *priv, > struct dpaa2_eth_channel *ch, > dma_addr_t addr) > { > + int retries = 0; > int err; > > ch->xdp.drop_bufs[ch->xdp.drop_cnt++] = addr; > @@ -229,8 +230,11 @@ static void xdp_release_buf(struct dpaa2_eth_priv *priv, > > while ((err = dpaa2_io_service_release(ch->dpio, priv->bpid, > ch->xdp.drop_bufs, > - ch->xdp.drop_cnt)) == -EBUSY) > + ch->xdp.drop_cnt)) == -EBUSY) { > + if (retries++ >= DPAA2_ETH_SWP_BUSY_RETRIES) > + break; > cpu_relax(); > + } > > if (err) { > free_bufs(priv, ch->xdp.drop_bufs, ch->xdp.drop_cnt); > @@ -458,7 +462,7 @@ static int consume_frames(struct dpaa2_eth_channel *ch, > struct dpaa2_eth_fq *fq = NULL; > struct dpaa2_dq *dq; > const struct dpaa2_fd *fd; > - int cleaned = 0; > + int cleaned = 0, retries = 0; > int is_last; > > do { > @@ -469,6 +473,11 @@ static int consume_frames(struct dpaa2_eth_channel *ch, > * the store until we get some sort of valid response > * token (either a valid frame or an "empty dequeue") > */ > + if (retries++ >= DPAA2_ETH_SWP_BUSY_RETRIES) { > + netdev_err_once(priv->net_dev, > + "Unable to read a valid dequeue response\n"); > + return 0; > + } > continue; Hi Ioana It seems a bit odd that here you could return -ETIMEDOUT, but you print an error, and return 0. But in the two cases above in void functions, you don't print anything. Please at least return -ETIMEDOUT when you can. Andrew