Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/amdkfd: Incomplete fix leaves lockless double-checked locking race --
commit f4e452c37b6ce531530ab68a693284bf20a378b1 Author: Henry Martin <[email protected]> drm/amdkfd: fix doorbell allocation race creating duplicate allocations The patch introduces a per-process-device mutex to serialize the allocation of doorbells in kfd_alloc_process_doorbells(). It removes the lockless check in kfd_get_process_doorbells() so that all synchronization is handled internally by the allocator. > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c > b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c > index fdcf7f2d1b5b4..6af268a031f22 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c > @@ -239,11 +239,9 @@ phys_addr_t kfd_get_process_doorbells(struct > kfd_process_device *pdd) > struct amdgpu_device *adev = pdd->dev->adev; > uint32_t first_db_index; > > - if (!pdd->qpd.proc_doorbells) { > - if (kfd_alloc_process_doorbells(pdd->dev->kfd, pdd)) > - /* phys_addr_t 0 is error */ > - return 0; > - } > + if (kfd_alloc_process_doorbells(pdd->dev->kfd, pdd)) > + /* phys_addr_t 0 is error */ > + return 0; [Severity: High] Are the lockless checks in kfd_ioctl_create_queue() and criu_restore_devices() intentionally left in drivers/gpu/drm/amd/amdkfd/kfd_chardev.c? While this lockless check was correctly removed here, identical lockless checks (if (!pdd->qpd.proc_doorbells)) remain in the other two call sites mentioned in the commit message. Concurrently, a thread calling kfd_mmap() will allocate the doorbell structures under doorbell_mutex. A racing thread in kfd_ioctl_create_queue() will read proc_doorbells locklessly. Without memory barriers like smp_load_acquire(), this thread can observe a non-NULL proc_doorbells pointer but fail to see the fully initialized qpd->doorbell_bitmap, or see the BO before it is fully pinned by amdgpu_bo_create(). Can this lead to a NULL pointer dereference in operations like find_first_zero_bit() on doorbell_bitmap, or hardware access with an unpinned BO? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
