Hi Dan,
Thanks for catching this. I'll check it and post a fix patch.
Regards,
Ma Jun
On 11/15/2022 9:00 PM, Dan Carpenter wrote:
> [ Ugh... My email messed up and I have to Resend all my emails for the
> past two weeks. -dan ]
>
> Hello Ma Jun,
>
> The patch c0cc999f3c32: "drm/amdkfd: Fix the warning of
> array-index-out-of-bounds" from Nov 2, 2022, leads to the following
> Smatch static checker warning:
>
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2008 kfd_topology_add_device()
> warn: inconsistent returns '&topology_lock'.
>
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c
> 1808 int kfd_topology_add_device(struct kfd_dev *gpu)
> 1809 {
> 1810 uint32_t gpu_id;
> 1811 struct kfd_topology_device *dev;
> 1812 struct kfd_cu_info cu_info;
> 1813 int res = 0;
> 1814 struct list_head temp_topology_device_list;
> 1815 void *crat_image = NULL;
> 1816 size_t image_size = 0;
> 1817 int proximity_domain;
> 1818 int i;
> 1819 const char *asic_name = amdgpu_asic_name[gpu->adev->asic_type];
> 1820
> 1821 INIT_LIST_HEAD(&temp_topology_device_list);
> 1822
> 1823 gpu_id = kfd_generate_gpu_id(gpu);
> 1824 pr_debug("Adding new GPU (ID: 0x%x) to topology\n", gpu_id);
> 1825
> 1826 /* Check to see if this gpu device exists in the topology_device_list.
> 1827 * If so, assign the gpu to that device,
> 1828 * else create a Virtual CRAT for this gpu device and then parse that
> 1829 * CRAT to create a new topology device. Once created assign the gpu to
> 1830 * that topology device
> 1831 */
> 1832 down_write(&topology_lock);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> Takes the lock.
>
> 1833 dev = kfd_assign_gpu(gpu);
> 1834 if (!dev) {
> 1835 proximity_domain = ++topology_crat_proximity_domain;
> 1836
> 1837 res = kfd_create_crat_image_virtual(&crat_image, &image_size,
> 1838 COMPUTE_UNIT_GPU, gpu,
> 1839 proximity_domain);
> 1840 if (res) {
> 1841 pr_err("Error creating VCRAT for GPU (ID: 0x%x)\n",
> 1842 gpu_id);
> 1843 topology_crat_proximity_domain--;
> 1844 return res;
>
> Does not drop the lock.
>
> 1845 }
> 1846
> 1847 res = kfd_parse_crat_table(crat_image,
> 1848 &temp_topology_device_list,
> 1849 proximity_domain);
> 1850 if (res) {
> 1851 pr_err("Error parsing VCRAT for GPU (ID: 0x%x)\n",
> 1852 gpu_id);
> 1853 topology_crat_proximity_domain--;
> 1854 goto err;
>
> Does not drop the lock.
>
> 1855 }
> 1856
> 1857 kfd_topology_update_device_list(&temp_topology_device_list,
> 1858 &topology_device_list);
> 1859
> 1860 dev = kfd_assign_gpu(gpu);
> 1861 if (WARN_ON(!dev)) {
> 1862 res = -ENODEV;
> 1863 goto err;
>
> Does not drop the lock.
>
> 1864 }
> 1865
> 1866 /* Fill the cache affinity information here for the GPUs
> 1867 * using VCRAT
> 1868 */
> 1869 kfd_fill_cache_non_crat_info(dev, gpu);
> 1870
> 1871 /* Update the SYSFS tree, since we added another topology
> 1872 * device
> 1873 */
> 1874 res = kfd_topology_update_sysfs();
> 1875 if (!res)
> 1876 sys_props.generation_count++;
> 1877 else
> 1878 pr_err("Failed to update GPU (ID: 0x%x) to sysfs topology. res=%d\n",
> 1879 gpu_id, res);
> 1880 }
> 1881 up_write(&topology_lock);
> ^^^^^^^^^^^^^^^^^^^^^^^^^
> Drops the lock. The patch has some other locking changes which are
> not explained in the commit message and seem unrelated to the out of
> bounds issue.
>
> 1882
> 1883 dev->gpu_id = gpu_id;
> 1884 gpu->id = gpu_id;
>
> regards,
> dan carpenter
>
