From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6973528DC3; Mon, 8 Jul 2024 07:46:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.188 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720424796; cv=none; b=B1HjHewm98MTKLJR35++2GihozNMG0nkvD+2TJ+gVCALVcy7hktU66DMaYrVqS/+xHTVBKXze7uiSRP+qkRi+BIQ5iBwFIuPeUSTLxhJL84anguQH6lUycnLk+avMEyyZSf0LDs0GyDvOrHhrh6B3/pbzEzKPN1kPQ8vV6bdV+w= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720424796; c=relaxed/simple; bh=69My5GLVF+kHwIfbakYCcE8YK5SoS9CHF5Q1fsCb0PA=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=bwWzv1l1RarMev3qoW4MFgS3GCtKHJlfyEolq9kD4TyL3R28oEXd3HyvMwxJmK2yBcMuJGbxd932bF7cUIUxanxJXxMBdwK/VS0K9RF5GoVhQa0rcJpMweaOEFGQAxBDj17wC8Hhy6Im/w7V+s8xY2GYxHZ8e1uvAL4gBhVY2EA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=hisilicon.com; spf=pass smtp.mailfrom=hisilicon.com; arc=none smtp.client-ip=45.249.212.188 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=hisilicon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=hisilicon.com Received: from mail.maildlp.com (unknown [172.19.88.105]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4WHblb3fCNznZfR; Mon, 8 Jul 2024 15:46:03 +0800 (CST) Received: from kwepemf100018.china.huawei.com (unknown [7.202.181.17]) by mail.maildlp.com (Postfix) with ESMTPS id C1B8D140485; Mon, 8 Jul 2024 15:46:27 +0800 (CST) Received: from [10.67.120.168] (10.67.120.168) by kwepemf100018.china.huawei.com (7.202.181.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 8 Jul 2024 15:46:27 +0800 Message-ID: <0bac285b-c8ae-8c9f-7c42-ee345f8682d1@hisilicon.com> Date: Mon, 8 Jul 2024 15:46:26 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.1.0 Subject: Re: [PATCH for-rc 2/9] RDMA/hns: Fix a long wait for cmdq event during reset Content-Language: en-US To: Leon Romanovsky CC: , , , References: <20240705085937.1644229-1-huangjunxian6@hisilicon.com> <20240705085937.1644229-3-huangjunxian6@hisilicon.com> <20240707083007.GE6695@unreal> <42e9f7dd-05bd-176f-c5c0-02e200b3f58c@hisilicon.com> <20240708053850.GA6788@unreal> <7cae577b-e469-9357-8375-d14746a7787b@hisilicon.com> <20240708073315.GC6788@unreal> From: Junxian Huang In-Reply-To: <20240708073315.GC6788@unreal> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemf100018.china.huawei.com (7.202.181.17) On 2024/7/8 15:33, Leon Romanovsky wrote: > On Mon, Jul 08, 2024 at 02:50:50PM +0800, Junxian Huang wrote: >> >> >> On 2024/7/8 13:38, Leon Romanovsky wrote: >>> On Mon, Jul 08, 2024 at 10:29:54AM +0800, Junxian Huang wrote: >>>> >>>> >>>> On 2024/7/7 16:30, Leon Romanovsky wrote: >>>>> On Fri, Jul 05, 2024 at 04:59:30PM +0800, Junxian Huang wrote: >>>>>> From: wenglianfa >>>>>> >>>>>> During reset, cmdq events won't be reported, leading to a long and >>>>>> unnecessary wait. Notify all the cmdqs to stop waiting at the beginning >>>>>> of reset. >>>>>> >>>>>> Fixes: 9a4435375cd1 ("IB/hns: Add driver files for hns RoCE driver") >>>>>> Signed-off-by: wenglianfa >>>>>> Signed-off-by: Junxian Huang >>>>>> --- >>>>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 18 ++++++++++++++++++ >>>>>> 1 file changed, 18 insertions(+) >>>>>> >>>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>>>>> index a5d746a5cc68..ff135df1a761 100644 >>>>>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>>>>> @@ -6977,6 +6977,21 @@ static void hns_roce_hw_v2_uninit_instance(struct hnae3_handle *handle, >>>>>> >>>>>> handle->rinfo.instance_state = HNS_ROCE_STATE_NON_INIT; >>>>>> } >>>>>> + >>>>>> +static void hns_roce_v2_reset_notify_cmd(struct hns_roce_dev *hr_dev) >>>>>> +{ >>>>>> + struct hns_roce_cmdq *hr_cmd = &hr_dev->cmd; >>>>>> + int i; >>>>>> + >>>>>> + if (!hr_dev->cmd_mod) >>>>> >>>>> What prevents cmd_mod from being changed? >>>>> >>>> >>>> It's set when the device is being initialized, and won't be changed after that. >>> >>> This is exactly the point, you are assuming that the device is already >>> ininitialized or not initialized at all. What prevents hns_roce_v2_reset_notify_cmd() >>> from being called in the middle of initialization? >>> >>> Thanks >>> >> >> This is ensured by hns3 NIC driver. >> >> Initialization and reset of hns RoCE are both called by hns3. It will check the state >> of RoCE device (see line 3798), and notify RoCE device to reset (hns_roce_v2_reset_notify_cmd() >> is called) only if the RoCE device has been already initialized: > > So why do you have "if (!hr_dev->cmd_mod)" check in the code? > > Thanks > cmd_mod indicates the mode of cmdq (0: poll mode, 1: event mode). This patch only affects event mode because HW won't report events during reset. Junxian >> >> 3791 static int hclge_notify_roce_client(struct hclge_dev *hdev, >> 3792 enum hnae3_reset_notify_type type) >> 3793 { >> 3794 struct hnae3_handle *handle = &hdev->vport[0].roce; >> 3795 struct hnae3_client *client = hdev->roce_client; >> 3796 int ret; >> 3797 >> 3798 if (!test_bit(HCLGE_STATE_ROCE_REGISTERED, &hdev->state) || !client) >> 3799 return 0; >> 3800 >> 3801 if (!client->ops->reset_notify) >> 3802 return -EOPNOTSUPP; >> 3803 >> 3804 ret = client->ops->reset_notify(handle, type); >> 3805 if (ret) >> 3806 dev_err(&hdev->pdev->dev, "notify roce client failed %d(%d)", >> 3807 type, ret); >> 3808 >> 3809 return ret; >> 3810 } >> >> And the bit is set (see line 11246) after the initialization has been done (line 11242): >> >> 11224 static int hclge_init_roce_client_instance(struct hnae3_ae_dev *ae_dev, >> 11225 struct hclge_vport *vport) >> 11226 { >> 11227 struct hclge_dev *hdev = ae_dev->priv; >> 11228 struct hnae3_client *client; >> 11229 int rst_cnt; >> 11230 int ret; >> 11231 >> 11232 if (!hnae3_dev_roce_supported(hdev) || !hdev->roce_client || >> 11233 !hdev->nic_client) >> 11234 return 0; >> 11235 >> 11236 client = hdev->roce_client; >> 11237 ret = hclge_init_roce_base_info(vport); >> 11238 if (ret) >> 11239 return ret; >> 11240 >> 11241 rst_cnt = hdev->rst_stats.reset_cnt; >> 11242 ret = client->ops->init_instance(&vport->roce); >> 11243 if (ret) >> 11244 return ret; >> 11245 >> 11246 set_bit(HCLGE_STATE_ROCE_REGISTERED, &hdev->state); >> 11247 if (test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state) || >> 11248 rst_cnt != hdev->rst_stats.reset_cnt) { >> 11249 ret = -EBUSY; >> 11250 goto init_roce_err; >> 11251 } >> >> Junxian >> >>>> >>>> Junxian >>>> >>>>>> + return; >>>>>> + >>>>>> + for (i = 0; i < hr_cmd->max_cmds; i++) { >>>>>> + hr_cmd->context[i].result = -EBUSY; >>>>>> + complete(&hr_cmd->context[i].done); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> static int hns_roce_hw_v2_reset_notify_down(struct hnae3_handle *handle) >>>>>> { >>>>>> struct hns_roce_dev *hr_dev; >>>>>> @@ -6997,6 +7012,9 @@ static int hns_roce_hw_v2_reset_notify_down(struct hnae3_handle *handle) >>>>>> hr_dev->dis_db = true; >>>>>> hr_dev->state = HNS_ROCE_DEVICE_STATE_RST_DOWN; >>>>>> >>>>>> + /* Complete the CMDQ event in advance during the reset. */ >>>>>> + hns_roce_v2_reset_notify_cmd(hr_dev); >>>>>> + >>>>>> return 0; >>>>>> } >>>>>> >>>>>> -- >>>>>> 2.33.0 >>>>>> >>>> >>