[PATCH 1/4] drm/amdgpu: Introduce gfx software ring(v3)

Luben Tuikov luben.tuikov at amd.com
Wed Sep 14 04:30:41 UTC 2022


On 2022-09-13 22:34, Zhu, Jiadong wrote:
>> +
>> +     r_rptr = amdgpu_ring_get_rptr(mux->real_ring);
>> +     r_wptr = amdgpu_ring_get_wptr(mux->real_ring);
>> These names are very much the same to a human. How about writep and readp?
> r_rptr for real ring's read ptr differed from sw_rptr. Maybe we change to real_rptr/real_wptr?
> 

"real_rptr" and "real_wptr" is the same as "r_rptr" and "r_wptr", and actually worse.
The problem is that there's too little entropy on those names and as such to a human
they look the same. In the current version of the patch, you have only one out of six
characters different and that's in the middle of the word--very hard for a human to see,
note and distinguish. The situation is even worse with "real_rptr" and "real_wptr",
as that's one out of nine characters different and still very hard for a human to
notice the difference.

For this reason I suggested, using "writep" and "readp" which are immediately
distinguishable from each other as they have high entropy. Now, what they mean,
you can put that in a comment, but please use names with high entropy in them,
i.e. they are different from each other and easily distinguishable by a human.

Regards,
-- 
Luben


More information about the amd-gfx mailing list