RE: [PATCH for-next 1/9] RDMA/hns: Refactor cmd init and mode selection for hip08

[liweihang <liweihang@hisilicon.com>]

> -----Original Message-----
> From: Linuxarm [mailto:linuxarm-bounces@huawei.com] On Behalf Of Leon
> Romanovsky
> Sent: Thursday, August 22, 2019 1:19 AM
> To: oulijun <oulijun@huawei.com>
> Cc: linux-rdma@vger.kernel.org; jgg@ziepe.ca; dledford@redhat.com;
> Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH for-next 1/9] RDMA/hns: Refactor cmd init and mode
> selection for hip08
> 
> On Wed, Aug 21, 2019 at 09:14:28PM +0800, Lijun Ou wrote:
> > From: Yixian Liu <liuyixian@huawei.com>
> >
> > This patch refactor the initialization of cmd, and also for the cmd
> > mode selection on event and poll mode.
> >
> > Signed-off-by: Yixian Liu <liuyixian@huawei.com>
> > Signed-off-by: Lang Chen <chenglang@huawei.com>
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_cmd.c  | 14 ++++----------
> > drivers/infiniband/hw/hns/hns_roce_main.c | 18 ++++++++----------
> >  2 files changed, 12 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c
> > b/drivers/infiniband/hw/hns/hns_roce_cmd.c
> > index ade26fa..547002f 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
> > @@ -251,23 +251,17 @@ int hns_roce_cmd_use_events(struct
> hns_roce_dev *hr_dev)
> >  	hr_cmd->token_mask = CMD_TOKEN_MASK;
> >  	hr_cmd->use_events = 1;
> >
> > -	down(&hr_cmd->poll_sem);
> > -
> >  	return 0;
> >  }
> >
> >  void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)  {
> >  	struct hns_roce_cmdq *hr_cmd = &hr_dev->cmd;
> > -	int i;
> > -
> > -	hr_cmd->use_events = 0;
> >
> > -	for (i = 0; i < hr_cmd->max_cmds; ++i)
> > -		down(&hr_cmd->event_sem);
> > -
> > -	kfree(hr_cmd->context);
> > -	up(&hr_cmd->poll_sem);
> > +	if (hr_cmd->use_events) {
> 
> Ensure that hr_cmd->context == NULL in places where it shouldn't be kfreed
> and remove this "if (hr_cmd->use_events)".
> 
> Thanks
> .

Hi Leon,

Thanks for your advice, will remove above judgement in v2.

> 
> 
> > +		kfree(hr_cmd->context);
> > +		hr_cmd->use_events = 0;
> > +	}
> >  }
> >
> >  struct hns_roce_cmd_mailbox
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c
> > b/drivers/infiniband/hw/hns/hns_roce_main.c
> > index 1b757cc..f3b2f67 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_main.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
> > @@ -902,6 +902,7 @@ int hns_roce_init(struct hns_roce_dev *hr_dev)
> >  		goto error_failed_cmd_init;
> >  	}
> >
> > +	/* EQ depends on poll mode, event mode depends on EQ */
> >  	ret = hr_dev->hw->init_eq(hr_dev);
> >  	if (ret) {
> >  		dev_err(dev, "eq init failed!\n");
> > @@ -911,8 +912,9 @@ int hns_roce_init(struct hns_roce_dev *hr_dev)
> >  	if (hr_dev->cmd_mod) {
> >  		ret = hns_roce_cmd_use_events(hr_dev);
> >  		if (ret) {
> > -			dev_err(dev, "Switch to event-driven cmd failed!\n");
> > -			goto error_failed_use_event;
> > +			dev_warn(dev,
> > +				 "Cmd event  mode failed, set back to
> poll!\n");
> > +			hns_roce_cmd_use_polling(hr_dev);
> >  		}
> >  	}
> >
> > @@ -928,12 +930,10 @@ int hns_roce_init(struct hns_roce_dev *hr_dev)
> >  		goto error_failed_setup_hca;
> >  	}
> >
> > -	if (hr_dev->hw->hw_init) {
> > -		ret = hr_dev->hw->hw_init(hr_dev);
> > -		if (ret) {
> > -			dev_err(dev, "hw_init failed!\n");
> > -			goto error_failed_engine_init;
> > -		}
> > +	ret = hr_dev->hw->hw_init(hr_dev);
> > +	if (ret) {
> > +		dev_err(dev, "hw_init failed!\n");
> > +		goto error_failed_engine_init;
> >  	}
> >
> >  	ret = hns_roce_register_device(hr_dev); @@ -955,8 +955,6 @@ int
> > hns_roce_init(struct hns_roce_dev *hr_dev)
> >  error_failed_init_hem:
> >  	if (hr_dev->cmd_mod)
> >  		hns_roce_cmd_use_polling(hr_dev);
> > -
> > -error_failed_use_event:
> >  	hr_dev->hw->cleanup_eq(hr_dev);
> >
> >  error_failed_eq_table:
> > --
> > 2.8.1
> >
> _______________________________________________
> Linuxarm mailing list
> Linuxarm@huawei.com
> http://hulk.huawei.com/mailman/listinfo/linuxarm