Message ID | 20211129140027.23036-2-huangguangbin2@huawei.com (mailing list archive) |
---|---|
State | Superseded |
Commit | ed0e658c51aadb64b871cfa3de7d26599f171cdb |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: hns3: some cleanups for -next | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Clearly marked for net-next |
netdev/apply | success | Patch already applied to net-next |
11/29/21 5:00 PM, Guangbin Huang пишет: > From: Jiaran Zhang <zhangjiaran@huawei.com> > > Currently, the hclge_reset_prepare_general function uses the goto > statement to jump upwards, which increases code complexity and makes > the program structure difficult to understand. In addition, if > reset_pending is set, retry_cnt cannot be increased. This may result > in a failure to exit the retry or increase the number of retries. > > Use the while statement instead to make the program easier to understand > and solve the problem that the goto statement cannot be exited. > > Signed-off-by: Jiaran Zhang <zhangjiaran@huawei.com> > Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com> > --- > .../hisilicon/hns3/hns3pf/hclge_main.c | 32 ++++++++----------- > .../hisilicon/hns3/hns3vf/hclgevf_main.c | 32 ++++++++----------- > 2 files changed, 28 insertions(+), 36 deletions(-) > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c > index a0628d139149..5282f2632b3b 100644 > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c > @@ -11589,24 +11589,20 @@ static void hclge_reset_prepare_general(struct hnae3_ae_dev *ae_dev, > int retry_cnt = 0; > int ret; > > -retry: > - down(&hdev->reset_sem); > - set_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); > - hdev->reset_type = rst_type; > - ret = hclge_reset_prepare(hdev); > - if (ret || hdev->reset_pending) { > - dev_err(&hdev->pdev->dev, "fail to prepare to reset, ret=%d\n", > - ret); > - if (hdev->reset_pending || > - retry_cnt++ < HCLGE_RESET_RETRY_CNT) { > - dev_err(&hdev->pdev->dev, > - "reset_pending:0x%lx, retry_cnt:%d\n", > - hdev->reset_pending, retry_cnt); > - clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); > - up(&hdev->reset_sem); > - msleep(HCLGE_RESET_RETRY_WAIT_MS); > - goto retry; > - } > + while (retry_cnt++ < HCLGE_RESET_RETRY_CNT) { > + down(&hdev->reset_sem); > + set_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); > + hdev->reset_type = rst_type; > + ret = hclge_reset_prepare(hdev); > + if (!ret && !hdev->reset_pending) > + break; up(&hdev->reset_sem); ? > + > + dev_err(&hdev->pdev->dev, > + "failed to prepare to reset, ret=%d, reset_pending:0x%lx, retry_cnt:%d\n", > + ret, hdev->reset_pending, retry_cnt); > + clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); > + up(&hdev->reset_sem); > + msleep(HCLGE_RESET_RETRY_WAIT_MS); > } > > /* disable misc vector before reset done */ > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c > index 3f29062eaf2e..0568cc31d391 100644 > --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c > @@ -2166,24 +2166,20 @@ static void hclgevf_reset_prepare_general(struct hnae3_ae_dev *ae_dev, > int retry_cnt = 0; > int ret; > > -retry: > - down(&hdev->reset_sem); > - set_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); > - hdev->reset_type = rst_type; > - ret = hclgevf_reset_prepare(hdev); > - if (ret) { > - dev_err(&hdev->pdev->dev, "fail to prepare to reset, ret=%d\n", > - ret); > - if (hdev->reset_pending || > - retry_cnt++ < HCLGEVF_RESET_RETRY_CNT) { > - dev_err(&hdev->pdev->dev, > - "reset_pending:0x%lx, retry_cnt:%d\n", > - hdev->reset_pending, retry_cnt); > - clear_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); > - up(&hdev->reset_sem); > - msleep(HCLGEVF_RESET_RETRY_WAIT_MS); > - goto retry; > - } > + while (retry_cnt++ < HCLGEVF_RESET_RETRY_CNT) { > + down(&hdev->reset_sem); > + set_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); > + hdev->reset_type = rst_type; > + ret = hclgevf_reset_prepare(hdev); > + if (!ret && !hdev->reset_pending) > + break; same here > + > + dev_err(&hdev->pdev->dev, > + "failed to prepare to reset, ret=%d, reset_pending:0x%lx, retry_cnt:%d\n", > + ret, hdev->reset_pending, retry_cnt); > + clear_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); > + up(&hdev->reset_sem); > + msleep(HCLGEVF_RESET_RETRY_WAIT_MS); > } > > /* disable misc vector before reset done */ >
On 2021/11/29 22:27, Denis Kirjanov wrote: > > > 11/29/21 5:00 PM, Guangbin Huang пишет: >> From: Jiaran Zhang <zhangjiaran@huawei.com> >> >> Currently, the hclge_reset_prepare_general function uses the goto >> statement to jump upwards, which increases code complexity and makes >> the program structure difficult to understand. In addition, if >> reset_pending is set, retry_cnt cannot be increased. This may result >> in a failure to exit the retry or increase the number of retries. >> >> Use the while statement instead to make the program easier to understand >> and solve the problem that the goto statement cannot be exited. >> >> Signed-off-by: Jiaran Zhang <zhangjiaran@huawei.com> >> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com> >> --- >> .../hisilicon/hns3/hns3pf/hclge_main.c | 32 ++++++++----------- >> .../hisilicon/hns3/hns3vf/hclgevf_main.c | 32 ++++++++----------- >> 2 files changed, 28 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c >> index a0628d139149..5282f2632b3b 100644 >> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c >> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c >> @@ -11589,24 +11589,20 @@ static void hclge_reset_prepare_general(struct hnae3_ae_dev *ae_dev, >> int retry_cnt = 0; >> int ret; >> -retry: >> - down(&hdev->reset_sem); >> - set_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); >> - hdev->reset_type = rst_type; >> - ret = hclge_reset_prepare(hdev); >> - if (ret || hdev->reset_pending) { >> - dev_err(&hdev->pdev->dev, "fail to prepare to reset, ret=%d\n", >> - ret); >> - if (hdev->reset_pending || >> - retry_cnt++ < HCLGE_RESET_RETRY_CNT) { >> - dev_err(&hdev->pdev->dev, >> - "reset_pending:0x%lx, retry_cnt:%d\n", >> - hdev->reset_pending, retry_cnt); >> - clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); >> - up(&hdev->reset_sem); >> - msleep(HCLGE_RESET_RETRY_WAIT_MS); >> - goto retry; >> - } >> + while (retry_cnt++ < HCLGE_RESET_RETRY_CNT) { >> + down(&hdev->reset_sem); >> + set_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); >> + hdev->reset_type = rst_type; >> + ret = hclge_reset_prepare(hdev); >> + if (!ret && !hdev->reset_pending) >> + break; > up(&hdev->reset_sem); ? This is reset preparation in PCIe FLR reset process, up(&hdev->reset_sem) will be called in hclge_reset_done(). >> + >> + dev_err(&hdev->pdev->dev, >> + "failed to prepare to reset, ret=%d, reset_pending:0x%lx, retry_cnt:%d\n", >> + ret, hdev->reset_pending, retry_cnt); >> + clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); >> + up(&hdev->reset_sem); >> + msleep(HCLGE_RESET_RETRY_WAIT_MS); >> } >> /* disable misc vector before reset done */ >> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c >> index 3f29062eaf2e..0568cc31d391 100644 >> --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c >> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c >> @@ -2166,24 +2166,20 @@ static void hclgevf_reset_prepare_general(struct hnae3_ae_dev *ae_dev, >> int retry_cnt = 0; >> int ret; >> -retry: >> - down(&hdev->reset_sem); >> - set_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); >> - hdev->reset_type = rst_type; >> - ret = hclgevf_reset_prepare(hdev); >> - if (ret) { >> - dev_err(&hdev->pdev->dev, "fail to prepare to reset, ret=%d\n", >> - ret); >> - if (hdev->reset_pending || >> - retry_cnt++ < HCLGEVF_RESET_RETRY_CNT) { >> - dev_err(&hdev->pdev->dev, >> - "reset_pending:0x%lx, retry_cnt:%d\n", >> - hdev->reset_pending, retry_cnt); >> - clear_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); >> - up(&hdev->reset_sem); >> - msleep(HCLGEVF_RESET_RETRY_WAIT_MS); >> - goto retry; >> - } >> + while (retry_cnt++ < HCLGEVF_RESET_RETRY_CNT) { >> + down(&hdev->reset_sem); >> + set_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); >> + hdev->reset_type = rst_type; >> + ret = hclgevf_reset_prepare(hdev); >> + if (!ret && !hdev->reset_pending) >> + break; > same here Same as above. >> + >> + dev_err(&hdev->pdev->dev, >> + "failed to prepare to reset, ret=%d, reset_pending:0x%lx, retry_cnt:%d\n", >> + ret, hdev->reset_pending, retry_cnt); >> + clear_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); >> + up(&hdev->reset_sem); >> + msleep(HCLGEVF_RESET_RETRY_WAIT_MS); >> } >> /* disable misc vector before reset done */ >> > .
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c index a0628d139149..5282f2632b3b 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c @@ -11589,24 +11589,20 @@ static void hclge_reset_prepare_general(struct hnae3_ae_dev *ae_dev, int retry_cnt = 0; int ret; -retry: - down(&hdev->reset_sem); - set_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); - hdev->reset_type = rst_type; - ret = hclge_reset_prepare(hdev); - if (ret || hdev->reset_pending) { - dev_err(&hdev->pdev->dev, "fail to prepare to reset, ret=%d\n", - ret); - if (hdev->reset_pending || - retry_cnt++ < HCLGE_RESET_RETRY_CNT) { - dev_err(&hdev->pdev->dev, - "reset_pending:0x%lx, retry_cnt:%d\n", - hdev->reset_pending, retry_cnt); - clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); - up(&hdev->reset_sem); - msleep(HCLGE_RESET_RETRY_WAIT_MS); - goto retry; - } + while (retry_cnt++ < HCLGE_RESET_RETRY_CNT) { + down(&hdev->reset_sem); + set_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); + hdev->reset_type = rst_type; + ret = hclge_reset_prepare(hdev); + if (!ret && !hdev->reset_pending) + break; + + dev_err(&hdev->pdev->dev, + "failed to prepare to reset, ret=%d, reset_pending:0x%lx, retry_cnt:%d\n", + ret, hdev->reset_pending, retry_cnt); + clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state); + up(&hdev->reset_sem); + msleep(HCLGE_RESET_RETRY_WAIT_MS); } /* disable misc vector before reset done */ diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c index 3f29062eaf2e..0568cc31d391 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c @@ -2166,24 +2166,20 @@ static void hclgevf_reset_prepare_general(struct hnae3_ae_dev *ae_dev, int retry_cnt = 0; int ret; -retry: - down(&hdev->reset_sem); - set_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); - hdev->reset_type = rst_type; - ret = hclgevf_reset_prepare(hdev); - if (ret) { - dev_err(&hdev->pdev->dev, "fail to prepare to reset, ret=%d\n", - ret); - if (hdev->reset_pending || - retry_cnt++ < HCLGEVF_RESET_RETRY_CNT) { - dev_err(&hdev->pdev->dev, - "reset_pending:0x%lx, retry_cnt:%d\n", - hdev->reset_pending, retry_cnt); - clear_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); - up(&hdev->reset_sem); - msleep(HCLGEVF_RESET_RETRY_WAIT_MS); - goto retry; - } + while (retry_cnt++ < HCLGEVF_RESET_RETRY_CNT) { + down(&hdev->reset_sem); + set_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); + hdev->reset_type = rst_type; + ret = hclgevf_reset_prepare(hdev); + if (!ret && !hdev->reset_pending) + break; + + dev_err(&hdev->pdev->dev, + "failed to prepare to reset, ret=%d, reset_pending:0x%lx, retry_cnt:%d\n", + ret, hdev->reset_pending, retry_cnt); + clear_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state); + up(&hdev->reset_sem); + msleep(HCLGEVF_RESET_RETRY_WAIT_MS); } /* disable misc vector before reset done */