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=-3.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 0B930C35254 for ; Wed, 5 Feb 2020 13:39:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D62BD20702 for ; Wed, 5 Feb 2020 13:39:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728150AbgBENj3 (ORCPT ); Wed, 5 Feb 2020 08:39:29 -0500 Received: from shards.monkeyblade.net ([23.128.96.9]:47128 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727367AbgBENj2 (ORCPT ); Wed, 5 Feb 2020 08:39:28 -0500 Received: from localhost (unknown [IPv6:2001:982:756:1:57a7:3bfd:5e85:defb]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) (Authenticated sender: davem-davemloft) by shards.monkeyblade.net (Postfix) with ESMTPSA id EE241158E83BF; Wed, 5 Feb 2020 05:39:25 -0800 (PST) Date: Wed, 05 Feb 2020 14:39:24 +0100 (CET) Message-Id: <20200205.143924.1875004608052019375.davem@davemloft.net> To: boon.leong.ong@intel.com Cc: netdev@vger.kernel.org, tee.min.tan@intel.com, weifeng.voon@intel.com, peppe.cavallaro@st.com, alexandre.torgue@st.com, Jose.Abreu@synopsys.com, mcoquelin.stm32@gmail.com, Joao.Pinto@synopsys.com, arnd@arndb.de, alexandru.ardelean@analog.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net v4 1/6] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues From: David Miller In-Reply-To: <20200205085510.32353-2-boon.leong.ong@intel.com> References: <20200205085510.32353-1-boon.leong.ong@intel.com> <20200205085510.32353-2-boon.leong.ong@intel.com> X-Mailer: Mew version 6.8 on Emacs 26.3 Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.12 (shards.monkeyblade.net [149.20.54.216]); Wed, 05 Feb 2020 05:39:28 -0800 (PST) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Ong Boon Leong Date: Wed, 5 Feb 2020 16:55:05 +0800 > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 5836b21edd7e..4d9afa13eeb9 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -2657,6 +2657,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp) > stmmac_enable_tbs(priv, priv->ioaddr, enable, chan); > } > > + /* Configure real RX and TX queues */ > + netif_set_real_num_rx_queues(dev, priv->plat->rx_queues_to_use); > + netif_set_real_num_tx_queues(dev, priv->plat->tx_queues_to_use); > + > /* Start the ball rolling... */ > stmmac_start_all_dma(priv); > It is only safe to ignore the return values from netif_set_real_num_{rx,tx}_queues() if you call them before the network device is registered. Because only in that case are these functions guaranteed to succeed. But now that you have moved these calls here, they can fail. Therefore you must check the return value and unwind the state completely upon failures. Honestly, I think this change will have several undesirable side effects: 1) Lots of added new code complexity 2) A new failure mode when resuming the device, users will find this very hard to diagnose and recover from What real value do you get from doing these calls after probe? If you can't come up with a suitable answer to that question, you should reconsider this change. Thanks. 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=-1.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,URIBL_DBL_ABUSE_MALW autolearn=no 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 C4F0DC35254 for ; Wed, 5 Feb 2020 13:39:44 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 98794217BA for ; Wed, 5 Feb 2020 13:39:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="oncE55kg" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 98794217BA Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=davemloft.net Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Mime-Version:References:In-Reply-To: From:Subject:To:Message-Id:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=VuZBb30y94QTXA7FTliPbrDU5Kph118kQN+QMjbrNaQ=; b=oncE55kg2LGX6O znvNTsdYEpvmsxuMjIw+FdUAnoVfCBStWh2Zx/vuuj+nwKFlpk31RGprdRWrqmf0zKbksHsRF2+4m ngTP6Pfpww1/A6ZltwhEBIgGn5wZx8CtbB1Is/s+J8KKJJZN1v6MN/gbIs52TuyNGM8WOP+BW8H/D RhPp3Dh9TWhWJp7SRxmugatuIKgZuDDNrXVpfZ8RiXozcb6KB1YjVgwoLCXX69WOcKhWDRfxfjCFQ XXw63QHBohjFOVhkS/Q1Alk+MteBhdWeFJFiA0ZJrT7EfOHCWUlpdfwRpZDfSCJDiP3dO0nnT2vAx KmKBwYl4d3IVEvcdc6yg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1izKu3-0004UN-B9; Wed, 05 Feb 2020 13:39:39 +0000 Received: from shards.monkeyblade.net ([2620:137:e000::1:9]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1izKtz-0004S7-RM for linux-arm-kernel@lists.infradead.org; Wed, 05 Feb 2020 13:39:37 +0000 Received: from localhost (unknown [IPv6:2001:982:756:1:57a7:3bfd:5e85:defb]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) (Authenticated sender: davem-davemloft) by shards.monkeyblade.net (Postfix) with ESMTPSA id EE241158E83BF; Wed, 5 Feb 2020 05:39:25 -0800 (PST) Date: Wed, 05 Feb 2020 14:39:24 +0100 (CET) Message-Id: <20200205.143924.1875004608052019375.davem@davemloft.net> To: boon.leong.ong@intel.com Subject: Re: [PATCH net v4 1/6] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues From: David Miller In-Reply-To: <20200205085510.32353-2-boon.leong.ong@intel.com> References: <20200205085510.32353-1-boon.leong.ong@intel.com> <20200205085510.32353-2-boon.leong.ong@intel.com> X-Mailer: Mew version 6.8 on Emacs 26.3 Mime-Version: 1.0 X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.12 (shards.monkeyblade.net [149.20.54.216]); Wed, 05 Feb 2020 05:39:28 -0800 (PST) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200205_053935_896624_489857C3 X-CRM114-Status: GOOD ( 11.95 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jose.Abreu@synopsys.com, Joao.Pinto@synopsys.com, alexandre.torgue@st.com, weifeng.voon@intel.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, arnd@arndb.de, mcoquelin.stm32@gmail.com, tee.min.tan@intel.com, peppe.cavallaro@st.com, alexandru.ardelean@analog.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Ong Boon Leong Date: Wed, 5 Feb 2020 16:55:05 +0800 > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 5836b21edd7e..4d9afa13eeb9 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -2657,6 +2657,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp) > stmmac_enable_tbs(priv, priv->ioaddr, enable, chan); > } > > + /* Configure real RX and TX queues */ > + netif_set_real_num_rx_queues(dev, priv->plat->rx_queues_to_use); > + netif_set_real_num_tx_queues(dev, priv->plat->tx_queues_to_use); > + > /* Start the ball rolling... */ > stmmac_start_all_dma(priv); > It is only safe to ignore the return values from netif_set_real_num_{rx,tx}_queues() if you call them before the network device is registered. Because only in that case are these functions guaranteed to succeed. But now that you have moved these calls here, they can fail. Therefore you must check the return value and unwind the state completely upon failures. Honestly, I think this change will have several undesirable side effects: 1) Lots of added new code complexity 2) A new failure mode when resuming the device, users will find this very hard to diagnose and recover from What real value do you get from doing these calls after probe? If you can't come up with a suitable answer to that question, you should reconsider this change. Thanks. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel