Hi, Leon and Jason:
Thanks a lot for yours reply.
On 2018/12/7 1:57, Jason Gunthorpe wrote:
> On Thu, Dec 06, 2018 at 08:49:43AM +0200, Leon Romanovsky wrote:
>
>>> "resp" point to "desc.data" which will be sent to hardware through
>>> function "hns_roce_cmq_send".
>>> resp->rocee_scc_ctx_clr_done will be set to 1 by hardware if
>>> scc context clear successful. If not, continue the query until the
>>> maximum number of queries is reached HNS_ROCE_CMQ_SCC_CLR_DONE_CNT.
>>
>> I'm not expert in this area, but it looks like you need some sort of
>> memory fence and ensure that memory in-sync between device and CPU.
>
> It looks like there is a readl on this path and readl is serializing
>
> Jason
>
As Jason said, there is a readl on this path and readl is serializing.
Thanks to your reminder, I found another problem. In a multi-concurrent
scenario, the scc ctx done flag may be overwritten by other threads.
This is because the hardware does not map the scc ctx done flag to qpn.
So spinlock is needed. The adjusted function is as follows:
+static int hns_roce_v2_qp_flow_control_init(struct hns_roce_dev *hr_dev,
+ struct hns_roce_qp *hr_qp)
+{
+ struct hns_roce_sccc_clr_done *rst, *resp;
+ struct hns_roce_sccc_clr *clr;
+ struct hns_roce_cmq_desc desc;
+ int ret, i;
+
+ spin_lock_bh(&hr_dev->qp_table.scc_lock);
+
+ /* set scc ctx clear done flag */
+ hns_roce_cmq_setup_basic_desc(&desc, HNS_ROCE_OPC_RESET_SCCC, false);
+ rst = (struct hns_roce_sccc_clr_done *)desc.data;
+ ret = hns_roce_cmq_send(hr_dev, &desc, 1);
+ if (ret) {
+ dev_err(hr_dev->dev, "reset SCC ctx failed(%d)\n", ret);
+ goto out;
+ }
+
+ /* clear scc context */
+ hns_roce_cmq_setup_basic_desc(&desc, HNS_ROCE_OPC_CLR_SCCC, false);
+ clr = (struct hns_roce_sccc_clr *)desc.data;
+ clr->qpn = cpu_to_le32(hr_qp->qpn);
+ ret = hns_roce_cmq_send(hr_dev, &desc, 1);
+ if (ret) {
+ dev_err(hr_dev->dev, "clear SCC ctx failed(%d)\n", ret);
+ goto out;
+ }
+
+ /* query scc context clear is done or not */
+ resp = (struct hns_roce_sccc_clr_done *)desc.data;
+ for (i = 0; i <= HNS_ROCE_CMQ_SCC_CLR_DONE_CNT; i++) {
+ hns_roce_cmq_setup_basic_desc(&desc,
+ HNS_ROCE_OPC_QUERY_SCCC, true);
+ ret = hns_roce_cmq_send(hr_dev, &desc, 1);
+ if (ret) {
+ dev_err(hr_dev->dev, "query clr cmq failed(%d)\n", ret);
+ goto out;
+ }
+
+ if (resp->clr_done) {
+ ret = 0;
+ goto out;
+ }
+
+ mdelay(1);
+ }
+
+ dev_err(hr_dev->dev, "query SCC clr done flag overtime.\n");
+ ret = -ETIMEDOUT;
+
+out:
+ spin_unlock_bh(&hr_dev->qp_table.scc_lock);
+ return ret;
+}
+
---
v4->v5:
1.Modify some variabile names in hns_roce_v2_qp_flow_control_init.
2.Redundant memset deletion in hns_roce_v2_qp_flow_control_init,
Because hns_roce_cmq_setup_basic_desc has zeroed out the desc.
3.Add spinlock to hns_roce_v2_qp_flow_control_init.
4.Add mdelay to query_scc_clr_done in hns_roce_v2_qp_flow_control_init.
5.Add dev_err in hns_roce_v2_qp_flow_control_init.
---
Thanks
>
