Coverity: gmc_v10_0_flush_gpu_tlb_pasid(): Program hangs

Hello!

This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20200424 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan

You're getting this email because you were associated with the identified lines of code (noted below) that were touched by commits:

Thu Apr 23 12:05:54 2020 +0800

    82478876eaac ("drm/amdgpu: protect ring overrun")

Coverity reported the following:

*** CID 1492715: Program hangs (LOCK) /drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c: 434 in gmc_v10_0_flush_gpu_tlb_pasid()

428     		kiq->pmf->kiq_invalidate_tlbs(ring,
429     					pasid, flush_type, all_hub);
430     		r = amdgpu_fence_emit_polling(ring, &seq, MAX_KIQ_REG_WAIT);
431     		if (r) {
432     			amdgpu_ring_undo(ring);
433     			spin_unlock(&kiq->ring_lock);
vvv     CID 1492715:  Program hangs  (LOCK)
vvv     Returning without unlocking "adev->gfx.kiq.ring_lock".
434     			return -ETIME;
435     		}
436
437     		amdgpu_ring_commit(ring);
438     		spin_unlock(&adev->gfx.kiq.ring_lock);
439     		r = amdgpu_fence_wait_polling(ring, seq, adev->usec_timeout);

If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):

Addresses-Coverity-ID: 1492715 ("Program hangs")

Thanks for your attention!

Coverity-bot

Hi

This is a false positive. Please have a look at the code, here &kiq->ring_lock is equal to &adev->gfx.kiq.ring_lock Because kiq is fetched from adev->gfx.kiq. These two expressions are equivalent.

Best Regards
Yintian Tao
-----Original Message-----
From: coverity-bot <[email protected]>
Sent: 2020年4月24日 23:26
To: Tao, Yintian <[email protected]>
Cc: Koenig, Christian <[email protected]>; Deucher, Alexander <[email protected]>; Gustavo A. R. Silva <[email protected]>; [email protected]
Subject: Coverity: gmc_v10_0_flush_gpu_tlb_pasid(): Program hangs

Hello!

This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20200424 as part of the linux-next scan project:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&amp;data=02%7C01%7CYintian.Tao%40amd.com%7Cb4e164ff3f1542f14c2008d7e863dae8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637233387886678181&amp;sdata=QyZO8PdGBQF6NrWhXb1Ym0aoQhzotuCzwT3M1TwYQfk%3D&amp;reserved=0

You're getting this email because you were associated with the identified lines of code (noted below) that were touched by commits:

Thu Apr 23 12:05:54 2020 +0800

    82478876eaac ("drm/amdgpu: protect ring overrun")

Coverity reported the following:

*** CID 1492715: Program hangs (LOCK) /drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c: 434 in gmc_v10_0_flush_gpu_tlb_pasid()

428     		kiq->pmf->kiq_invalidate_tlbs(ring,
429     					pasid, flush_type, all_hub);
430     		r = amdgpu_fence_emit_polling(ring, &seq, MAX_KIQ_REG_WAIT);
431     		if (r) {
432     			amdgpu_ring_undo(ring);
433     			spin_unlock(&kiq->ring_lock);
vvv     CID 1492715:  Program hangs  (LOCK)
vvv     Returning without unlocking "adev->gfx.kiq.ring_lock".
434     			return -ETIME;
435     		}
436
437     		amdgpu_ring_commit(ring);
438     		spin_unlock(&adev->gfx.kiq.ring_lock);
439     		r = amdgpu_fence_wait_polling(ring, seq, adev->usec_timeout);

If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):

Addresses-Coverity-ID: 1492715 ("Program hangs")

Thanks for your attention!

--
Coverity-bot

Hi guys,

yes Yintian is right this is essentially the same lock just different pointers to it.

Looks like that Coverity is not doing an alias analyses to figure out if two pointer point to the same thing.

On the other hand we could probably use the same pointer in the code for both locking and unlocking.

Regards,
Christian.

Am 25.04.20 um 08:46 schrieb Tao, Yintian: