Hi Iago, Thank you for finding and reporting this potential double lock.
Yes indeed, I see it--it can indeed happen. Now, since the primitives used--macros using "amdgpu_mm_(r|w)reg\(.*\)"--in "amdgpu_device_vram_access()" do use their own register-access spinlocks, it maybe wise to remove the spinlock take/release in "amdgpu_device_vram_access()". We'll look into it and possibly submit another patch. Thanks again. Regards, Luben On 2019-11-16 11:21 a.m., Iago Abal wrote: > Hi, > > With the help of a static bug finder (EBA - https://github.com/IagoAbal/eba) > I have found a potential double lock in Linux Next tag next-20191115, file > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c. > > This bug seems to be introduced by commit > e35e2b117f4 ("drm/amdgpu: add a generic fb accessing helper function(v3)"). > > The steps to reproduce it would be: > > 1. Start in function `amdgpu_device_vram_access`. > 2. Enter for-loop `for (last += pos; pos <= last; pos += 4)`. > 3. First lock: `spin_lock_irqsave(&adev->mmio_idx_lock, flags)`. > 4. Call to `WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)pos) | 0x80000000)`. > 5. Note `#define WREG32_NO_KIQ(reg, v) amdgpu_mm_wreg(adev, (reg), (v), > AMDGPU_REGS_NO_KIQ)`. > 6. Continue in function `amdgpu_mm_wreg`. > 7. Take else-branch in the third if-statement. > 8. Double lock: `spin_lock_irqsave(&adev->mmio_idx_lock, flags)`. > > I think the control flow could reach that second lock, but you may know > better. > > Hope it helps! > > -- iago _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
