Received-SPF: pass (google.com: domain of linux-kernel+bounces-104747-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 15 Mar 2024 18:48:43 +0100
From: Michal Kubiak <michal.kubiak@intel.com>
To: Jijie Shao <shaojijie@huawei.com>
CC: <yisen.zhuang@huawei.com>, <salil.mehta@huawei.com>,
	<davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
	<pabeni@redhat.com>, <shenjian15@huawei.com>, <wangjie125@huawei.com>,
	<liuyonglong@huawei.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net 2/3] net: hns3: fix kernel crash when devlink reload
 during pf initialization
Message-ID: <ZfSJ+08L+Tr3zeGU@localhost.localdomain>
References: <20240315100748.2913882-1-shaojijie@huawei.com>
 <20240315100748.2913882-3-shaojijie@huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240315100748.2913882-3-shaojijie@huawei.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?+ctEiPkPevlabT1a1mxqGdwn3+2JrYQhXGQuF15gVhuIoywd6ClrIfgHWreA?=
 =?us-ascii?Q?1rk4RGpyLW24RmhPF3UPQD47ACVzOsTRucZz+E+njGzRDLfU/Gwpp2siM3PW?=
 =?us-ascii?Q?h8jqbWi9Vf0xtUGHe1WfLVsmZ8JcIBtNYBBUGrYbpjBETHJT6hL2mD3KXcWn?=
 =?us-ascii?Q?FGY68iGT0i8PG0GdiOynrAalxGnBkuZ4TYVzLUmZ/cUcDk1P+b9q/CHRS1kX?=
 =?us-ascii?Q?ofmrQtmH3EeJ5/3zJxVnaLR053df3x4DZlrWXHzys3GCUUbC5ZL3bUvCZjLk?=
 =?us-ascii?Q?lqrcOK58xTUoJjkF65kwkotUPRzK1T1XN87/wy7iYwg5YmrTLUwUVDs41Uw4?=
 =?us-ascii?Q?bjBiTdLjTdcjEisdyVclNXTEig/qGBJSI2QVsf7/uF17mqYVNPiQ1flyLJOP?=
 =?us-ascii?Q?G4p6Jg9zAZwcUfeMCHbFDrnbIbz/NEYLH7yWNJUdhHA/VUEfgko/v5rq0xau?=
 =?us-ascii?Q?tEGp6/8rzAc5YHdL5ODzm9WmaMx0HsL4S7wis5nOLKCpO/jMSfJAkI8DSpQx?=
 =?us-ascii?Q?lLMj+2CJM6qt/LD8q5GaxD4Bsq6Ji4szZiHnv3293FwtzIorpkaSAJPMS+1n?=
 =?us-ascii?Q?bVZk+VaxU3pOa0CWuxucTm7Qpt2h2nTHmQStEU1bn10RvxzmG88tXm1ZX+hb?=
 =?us-ascii?Q?SKxrYI2Q9QUj7iuRYmTyhcx87NwyZrqK17IlwvkFLg17filg/oQdShPCa3cC?=
 =?us-ascii?Q?UgTZefxc4tOF3XSjLaeQ6CX+5E4KVfLt6aSaRUPeHWsDf+my1WoLM6PgWWK1?=
 =?us-ascii?Q?GIxFR2SKvGjFxJ8OlpST3LCeuFjHQr5iCT/vcJO474gdN8G1PC++EXv6y+/g?=
 =?us-ascii?Q?QcOBVvfxywKrR7LTCrmHPq0LCdq67v9CgzTCLShC73y0uzS7Yeo9chRGfH7i?=
 =?us-ascii?Q?PyEPiMzHJ2TVb0qOUUvmZR55LByvSV31UqNeL7vCPqiYuL+ZrcsJN7O1uEV4?=
 =?us-ascii?Q?iTpYLZ4loCCdUiBTy/Aw/LOtQjMF6G/2019uiUhdBn1ZuTRH3t3p9WSOn9ES?=
 =?us-ascii?Q?DwPVA4u6HFL4AM9lEz/1RyDRIPeSEfGcVzOXmhVRGVWk+xmOVzYoBKRgry/0?=
 =?us-ascii?Q?KwkDgzhnLgGmwLmSme5rgQnYmvsbJ40md4B2bcdBJsVkwlKMA1CtgXuwXLn0?=
 =?us-ascii?Q?CFwrdiojjq+ehTdqAnOZudKC+vpYzq4zcTCAKFdN6CfoBctTTVwMzLEcMuwY?=
 =?us-ascii?Q?9QsdfvSjRUn4UZWHIqjbrFv/QObz7DdPPzGjydsJhOq5WRNxAHZrnponzNoU?=
 =?us-ascii?Q?1vJn1+nYKsPvL6Ndncs66YKpf9kQyHyUUdDV1Bh1AXJ04MV3cCaM8uH/DuKZ?=
 =?us-ascii?Q?tCCdnXoIE+JeHLxe/iqUmW0KxceRglAg8nGzlsEcsRyn6PjHhAYR0+YpV3g7?=
 =?us-ascii?Q?93WqNFl0VzR10hrWhKxf6xOZST1G8FRw8uVUwMiK64bmoG7tcCR/K13A5Yzd?=
 =?us-ascii?Q?IOtQsPxudqmPie+0c07VBtu62u8Nh2seDZywar3Ax+5eYlxJxmGy7Kc1qo5V?=
 =?us-ascii?Q?Mn4r6NlTpII+Gr7iBLPh6lgo0iQlnWgM+yVi7G9RnDRTfjoooTnqU0ONGMnl?=
 =?us-ascii?Q?0zzJ+vDZa5IeKYz/3cH70abKNX13gwwr7LALJeiAKXnxf2ujD7BoJNIA2aRT?=
 =?us-ascii?Q?uQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: cc2d3b77-6d3a-4dbc-a2a0-08dc45182f84
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5782.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Mar 2024 17:48:55.9818
 (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: iDUPnAx1xPTdpGehiGUlx5Z/7KIcU/D0bKqGE1/BzrAFNIIa+dfll7tWopVR6ZLWGLGf9aOsbB+vC97kNoyqtA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7571
X-OriginatorOrg: intel.com

On Fri, Mar 15, 2024 at 06:07:47PM +0800, Jijie Shao wrote:
> From: Yonglong Liu <liuyonglong@huawei.com>
> 
> The devlink reload process will access the hardware resources,
> but the register operation is before the hardware initialize
> done, so, if process the devlink reload during initializetion,

typo: initializetion -> initialization
Moreover, I would suggest to fix the syntax of that description because
it seems to be not obvious.

e.g. the register operation is done before the hardware is initialized
(but I may not get your intention)

> may lead the kernel crash. This patch check whether the NIC is

-> may lead to kernel crash

> initialized to fix the problem.

Maybe just: "Fix this by adding the check if the NIC initialization check" ?

> 
> Fixes: b741269b2759 ("net: hns3: add support for registering devlink for PF")
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
> index 9a939c0b217f..80db4f7b05f6 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
> @@ -40,8 +40,9 @@ static int hclge_devlink_reload_down(struct devlink *devlink, bool netns_change,
>  	struct pci_dev *pdev = hdev->pdev;
>  	int ret;
>  
> -	if (test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state)) {
> -		dev_err(&pdev->dev, "reset is handling\n");
> +	if (test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state) ||
> +	    !test_bit(HCLGE_STATE_NIC_REGISTERED, &hdev->state)) {
> +		dev_err(&pdev->dev, "reset is handling or driver removed\n");
>  		return -EBUSY;
>  	}
>  
> -- 
> 2.30.0
> 
> 


Thanks,
Michal