Re: [PATCH net-next 01/10] net: hns3: refactor reset_prepare_general retry statement

[huangguangbin (A)]

On 2021/11/29 22:27, Denis Kirjanov wrote:
> 11/29/21 5:00 PM, Guangbin Huang пишет:
> > From: Jiaran Zhang <zhangjiaran@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > Signed-off-by: Guangbin Huang <huangguangbin2@xxxxxxxxxx>
> > ---
> >   .../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 */
> .