Christian König <christian.koenig at amd.com> writes: > Am 27.04.2017 um 18:17 schrieb Nikola Pajkovsky: >> This is super simple elimination of else branch and I should >> probably even use unlikely in >> >> if (ring->count_dw < count_dw) { >> >> However, amdgpu_ring_write() has similar if condition, but does not >> return after DRM_ERROR and it looks suspicious. On error, we still >> adding v to ring and keeping count_dw-- below zero. >> >> if (ring->count_dw <= 0) >> DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n"); >> ring->ring[ring->wptr++] = v; >> ring->wptr &= ring->ptr_mask; >> ring->count_dw--; >> >> I can obviously be totaly wrong. Hmm? > > That's just choosing the lesser evil. > > When we write more DW to the ring than expected it is possible (but > not likely) that we override stuff on the ring buffer which is still > executed by the command processor leading to a possible CP crash. > > But when we completely drop the write the commands in the ring buffer > will certainly be invalid and so the CP will certainly crash sooner or > later. Instead of choosing the lesser evil, is there good way to design ring buffer right way? > Please add the unlikely() as well and then send out the patch with a > signed-of-by line and I will be happy to push it into our upstream > branch. Proper patch has been sent. -- Nikola