[bug report] RDMA/hns: Optimize cmd init and mode selection for hip08

[Dan Carpenter <dan.carpenter@oracle.com>]

Hello Yixian Liu,

The patch 3d50503b3b33: "RDMA/hns: Optimize cmd init and mode
selection for hip08" from Aug 29, 2019, leads to the following static
checker warning:

	drivers/infiniband/hw/hns/hns_roce_main.c:926 hns_roce_init()
	error: double unlocked '&hr_dev->cmd.poll_sem' (orig line 879)

drivers/infiniband/hw/hns/hns_roce_main.c
    833 int hns_roce_init(struct hns_roce_dev *hr_dev)
    834 {
    835 	struct device *dev = hr_dev->dev;
    836 	int ret;
    837 
    838 	if (hr_dev->hw->reset) {
    839 		ret = hr_dev->hw->reset(hr_dev, true);
    840 		if (ret) {
    841 			dev_err(dev, "Reset RoCE engine failed!\n");
    842 			return ret;
    843 		}
    844 	}
    845 	hr_dev->is_reset = false;
    846 
    847 	if (hr_dev->hw->cmq_init) {
    848 		ret = hr_dev->hw->cmq_init(hr_dev);
    849 		if (ret) {
    850 			dev_err(dev, "Init RoCE Command Queue failed!\n");
    851 			goto error_failed_cmq_init;
    852 		}
    853 	}
    854 
    855 	ret = hr_dev->hw->hw_profile(hr_dev);
    856 	if (ret) {
    857 		dev_err(dev, "Get RoCE engine profile failed!\n");
    858 		goto error_failed_cmd_init;
    859 	}
    860 
    861 	ret = hns_roce_cmd_init(hr_dev);
    862 	if (ret) {
    863 		dev_err(dev, "cmd init failed!\n");
    864 		goto error_failed_cmd_init;
    865 	}
    866 
    867 	/* EQ depends on poll mode, event mode depends on EQ */
    868 	ret = hr_dev->hw->init_eq(hr_dev);
    869 	if (ret) {
    870 		dev_err(dev, "eq init failed!\n");
    871 		goto error_failed_eq_table;
    872 	}
    873 
    874 	if (hr_dev->cmd_mod) {
    875 		ret = hns_roce_cmd_use_events(hr_dev);

If hns_roce_cmd_use_events() fails then that means we haven't taken the
semaphore.

    876 		if (ret) {
    877 			dev_warn(dev,
    878 				 "Cmd event  mode failed, set back to poll!\n");
    879 			hns_roce_cmd_use_polling(hr_dev);
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This releases a semaphore but we are not holding it.


    880 		}
    881 	}
    882 
    883 	ret = hns_roce_init_hem(hr_dev);
    884 	if (ret) {
    885 		dev_err(dev, "init HEM(Hardware Entry Memory) failed!\n");
    886 		goto error_failed_init_hem;
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
Let's assume we hit this goto

    887 	}
    888 
    889 	ret = hns_roce_setup_hca(hr_dev);
    890 	if (ret) {
    891 		dev_err(dev, "setup hca failed!\n");
    892 		goto error_failed_setup_hca;
    893 	}
    894 
    895 	if (hr_dev->hw->hw_init) {
    896 		ret = hr_dev->hw->hw_init(hr_dev);
    897 		if (ret) {
    898 			dev_err(dev, "hw_init failed!\n");
    899 			goto error_failed_engine_init;
    900 		}
    901 	}
    902 
    903 	INIT_LIST_HEAD(&hr_dev->qp_list);
    904 	spin_lock_init(&hr_dev->qp_list_lock);
    905 	INIT_LIST_HEAD(&hr_dev->dip_list);
    906 	spin_lock_init(&hr_dev->dip_list_lock);
    907 
    908 	ret = hns_roce_register_device(hr_dev);
    909 	if (ret)
    910 		goto error_failed_register_device;
    911 
    912 	return 0;
    913 
    914 error_failed_register_device:
    915 	if (hr_dev->hw->hw_exit)
    916 		hr_dev->hw->hw_exit(hr_dev);
    917 
    918 error_failed_engine_init:
    919 	hns_roce_cleanup_bitmap(hr_dev);
    920 
    921 error_failed_setup_hca:
    922 	hns_roce_cleanup_hem(hr_dev);
    923 
    924 error_failed_init_hem:
    925 	if (hr_dev->cmd_mod)
--> 926 		hns_roce_cmd_use_polling(hr_dev);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This will release the semaphore a second time again.

    927 	hr_dev->hw->cleanup_eq(hr_dev);
    928 
    929 error_failed_eq_table:
    930 	hns_roce_cmd_cleanup(hr_dev);
    931 
    932 error_failed_cmd_init:
    933 	if (hr_dev->hw->cmq_exit)
    934 		hr_dev->hw->cmq_exit(hr_dev);
    935 
    936 error_failed_cmq_init:
    937 	if (hr_dev->hw->reset) {
    938 		if (hr_dev->hw->reset(hr_dev, false))
    939 			dev_err(dev, "Dereset RoCE engine failed!\n");
    940 	}
    941 
    942 	return ret;
    943 }

regards,
dan carpenter