On 2021/7/28 19:43, Dan Carpenter wrote:
> 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
> .
>
Thank you for reporting this bug and I will submit a patch to fix it
as soon as possible.
regards,
Wenpeng Liang
