Date:   Tue, 14 Mar 2023 10:22:07 +0100
From:   Piotr Raczynski <piotr.raczynski@intel.com>
To:     Serge Semin <Sergey.Semin@baikalelectronics.ru>
CC:     Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        "Jakub Kicinski" <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "Andrew Lunn" <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Joao Pinto <Joao.Pinto@synopsys.com>,
        Serge Semin <fancer.lancer@gmail.com>,
        "Alexey Malahov" <Alexey.Malahov@baikalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
        Christian Marangi <ansuelsmth@gmail.com>,
        Biao Huang <biao.huang@mediatek.com>,
        Yang Yingliang <yangyingliang@huawei.com>,
        <netdev@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net 07/13] net: stmmac: Free Rx descs on Tx allocation
 failure
Message-ID: <ZBA8v592HgowIs6R@nimitz>
References: <20230313224237.28757-1-Sergey.Semin@baikalelectronics.ru>
 <20230313224237.28757-8-Sergey.Semin@baikalelectronics.ru>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230313224237.28757-8-Sergey.Semin@baikalelectronics.ru>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?CwWhxQkjpCl7OKNNyGzKd2yoZEstJ0uS8pG22yVx9dFHtWkbnfra6fBJdRvj?=
 =?us-ascii?Q?1IiMcejiysxVWMG0NwIJJzz/wWVfmNQuaM9F4tW2jgIXqOw2LccrXRAdcFOu?=
 =?us-ascii?Q?c2YFzYYGAkT0P1bFNZHuDOS5f00owRAH3YzgbjfUgEhqLVQIM0eP2ndBZTSM?=
 =?us-ascii?Q?BIlMz/m58A1CV/VQt/pNmMaQAzjWXR2iZ0jGXSJP/9oaB9rKl72XPb/BrjhQ?=
 =?us-ascii?Q?13cMAQH2ZEOf6rBWzQwId0Ob0fZUQclzyU67tJG/152lsF0kQtBe7cQAApy/?=
 =?us-ascii?Q?T91V9fmyoJsSpjqX1kNqUpYlv6VJhlH+1yRCOzCxXlI1f+WK5nlXJhwBKJ5N?=
 =?us-ascii?Q?6JYKgSseHy4w1NqCoEBe+NT9Pxo3F/q8k+OVqdWfp342LoTwGV3gZSZl9kTn?=
 =?us-ascii?Q?VBVUCYkHxcWDliroUsr7OA6VSQT/ZNgI02MLsW+ilvdufoEB3vxRClF5O9tb?=
 =?us-ascii?Q?DZoGLhWyiue9WhllVyzxrJIHIb26Gv3Ps/Y8qk0ClGsd7RkRihg4zHrMluK1?=
 =?us-ascii?Q?+GVjGAF1Nik8XL2nzBzxMflkIFt673WB/wpeiBAOhSk+PNXx+r90dnPRVAqM?=
 =?us-ascii?Q?jbGLXaWK94qW4enhta0qdE4UhbTVxgp8QlC2YWw/VBJUIJE/wttJMqBStXAO?=
 =?us-ascii?Q?orVkkIr2N1QfN3B5AA1hgk6NX+7lJ99zQ8NvuBa7ylsFfXLNOVxsFDFOnV62?=
 =?us-ascii?Q?fR74hqM8sA1/GS/78kygVB56G4AawcRXm1M7GGTzmQ+/66N6ry/ZXT+fwcJ8?=
 =?us-ascii?Q?XP6af7phYv7/ELP15MbCW7lb6vM7KlCWrzFE+Btg7unThrWLOviT+YWOzMZc?=
 =?us-ascii?Q?TN2dwhlHTpyAWdroumKOZxdO7kn4Ls++UeQZbUbbixuLnonODNnPyehg9XHF?=
 =?us-ascii?Q?XpMTxj/14DvRYg2kI/s8Ir6flL6yo1Fi8mibbq/jC1peEw6x9HtbGYNHjRq8?=
 =?us-ascii?Q?FAECXNPcBrdW+ty3B2tOWewes6oIHc5I93hP8LZjPKjjUG7kJuKaiK05herN?=
 =?us-ascii?Q?YiyTyMd5CMtwFw/D37brSwmBFy5FcyBzMyejy79eJLPxtExCy3qghwbB/k78?=
 =?us-ascii?Q?GJKDJoMqHigE7yJ1Y0GyAM1auIGthhOtt0qLmAVUJ1932GeRDRU8MQc73T4M?=
 =?us-ascii?Q?7n1gKWh0AjJTtwCubv0kgK8odjFLx3GIe+M7m0BTFvFNxGS7Qvf8XOzwuq0A?=
 =?us-ascii?Q?q80JGhMvWTIT8Ab6LtT4os8VQmHlLFUlVgZxoW/WuYxWH/7CKSGkuWTH4O5E?=
 =?us-ascii?Q?TFJHqmwYo924Ffz0vtln1iIyn0+uBZ6Mhpql6Cp/6A9JkkSvwbEBuFNIVIw3?=
 =?us-ascii?Q?znSM1M+oXvC1GTu3FMu2WsfHu/Zah4vo+yLkVTvcVZgoI3EEgCSqwBQ7wZK1?=
 =?us-ascii?Q?RWTK/pzhi/FfXJtjwY2RQRovgFPHNb3+E1Y109H5tnWQwWYUthlTOE5Oxq9P?=
 =?us-ascii?Q?NfKvrRQuFQDVGqg3TWS5Qk8gYSNpCkYxSVAwtyy+3AA+wS7FVjVuvYBAg2jJ?=
 =?us-ascii?Q?/p9hL4bp+b4JcT2KSGz+2gMrR8gGnOCK7ZuAqmlSa2FcDnXYCc5x0LRcF7gM?=
 =?us-ascii?Q?T60HfiuP3lXrY8U0vyp6UIl+NQfUIq/T1aF4OhPQut6YEgxVPTWDSX0wdP0V?=
 =?us-ascii?Q?lQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6583c6a2-28b6-4920-32c8-08db246d98e5
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB7471.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2023 09:22:13.9812
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wEohB9unlS+uhGzEotYPwonhoNosbJim+nvYvz1OPqtMY0YTkbr0o3X0R7lTrMnG/SUwX7PYaRCgcqHjNyepbX1+9RMlVORQrPFrDuesS4Y=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5090
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Mar 14, 2023 at 01:42:31AM +0300, Serge Semin wrote:
> Indeed in accordance with the alloc_dma_desc_resources() method logic the
> Rx descriptors will be left allocated if Tx descriptors allocation fails.
> Fix it by calling the free_dma_rx_desc_resources() in case if the
> alloc_dma_tx_desc_resources() method returns non-zero value.
> 
> While at it refactor the method a bit. Just move the Rx descriptors
> allocation method invocation out of the local variables declaration block
> and discard a pointless comment from there.
> 

LGTM, Thanks.
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>
> Fixes: 71fedb0198cb ("net: stmmac: break some functions into RX and TX scopes")
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 4d643b1bbf65..229f827d7572 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2182,13 +2182,15 @@ static int alloc_dma_tx_desc_resources(struct stmmac_priv *priv,
>  static int alloc_dma_desc_resources(struct stmmac_priv *priv,
>  				    struct stmmac_dma_conf *dma_conf)
>  {
> -	/* RX Allocation */
> -	int ret = alloc_dma_rx_desc_resources(priv, dma_conf);
> +	int ret;
>  
> +	ret = alloc_dma_rx_desc_resources(priv, dma_conf);
>  	if (ret)
>  		return ret;
>  
>  	ret = alloc_dma_tx_desc_resources(priv, dma_conf);
> +	if (ret)
> +		free_dma_rx_desc_resources(priv, dma_conf);
>  
>  	return ret;
>  }
> -- 
> 2.39.2
> 
>