Received-SPF: pass (google.com: domain of linux-kernel+bounces-155239-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 23 Apr 2024 15:00:35 +0200
From: Jiri Pirko <jiri@resnulli.us>
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, chenhao418@huawei.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net 7/7] net: hns3: fix kernel crash when devlink reload
 during vf initialization
Message-ID: <Ziew8wLJ180m9PR6@nanopsycho>
References: <20240422134327.3160587-1-shaojijie@huawei.com>
 <20240422134327.3160587-8-shaojijie@huawei.com>
 <ZiZx0E5NlSL7nuLE@nanopsycho>
 <2ada3cd6-ed7d-432f-8aa4-cbeafa820590@huawei.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2ada3cd6-ed7d-432f-8aa4-cbeafa820590@huawei.com>

Tue, Apr 23, 2024 at 01:51:21PM CEST, shaojijie@huawei.com wrote:
>
>on 2024/4/22 22:18, Jiri Pirko wrote:
>> Mon, Apr 22, 2024 at 03:43:27PM CEST,shaojijie@huawei.com  wrote:
>> > From: Yonglong Liu<liuyonglong@huawei.com>
>> > 
>> > The devlink reload process will access the hardware resources,
>> > but the register operation is done before the hardware is initialized.
>> > So, processing the devlink reload during initialization may lead to kernel
>> > crash. This patch fixes this by taking devl_lock during initialization.
>> > 
>> > Fixes: cd6242991d2e ("net: hns3: add support for registering devlink for VF")
>> > Signed-off-by: Yonglong Liu<liuyonglong@huawei.com>
>> > Signed-off-by: Jijie Shao<shaojijie@huawei.com>
>> > ---
>> > drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 4 ++++
>> > 1 file changed, 4 insertions(+)
>> > 
>> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
>> > index 08db8e84be4e..3ee41943d15f 100644
>> > --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
>> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
>> > @@ -2849,6 +2849,8 @@ static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
>> > 	if (ret)
>> > 		goto err_devlink_init;
>> > 
>> Hmm, what if reload happens here? I think that you don't fix the issue,
>> rather just narrowing the race window.
>
>Yes the issue still occurs.
>
>> Why don't you rather calle devlink_register at the end of this function?
>
>We intended to fix this issue with minimal changes.

I'm not suggesting anything that would mean the opposite.


>But now it seems that we can only call devlink_register at the end of this function

I don't follow. Is that a question? That is what I suggested.


>
>> Also, parallel to this patch, why don't you register devlink port of
>> flavour "virtual" for this VF?
>
>I'm sorry, I'm not sure what "flavour "virtual"" means?

git grep DEVLINK_PORT_FLAVOUR_VIRTUAL


>And How can we use it? If it is useful, I can send another patch to optimize the code.

Please do.


>
>Tnanks,
>Jijie Shao