On Fri, Jun 23, 2017 at 3:45 PM, axie <a...@amd.com> wrote: > Hi Marek, > > I understand you spent time on your original logic too. I really don't > understand why you talked about pain if somebody can improve it. > > To reduce the pain, now I am seriously considering dropping this patch. But > please read on before you conclude. Let us treat open source software > development a fun. > > Same trick like this patch could be found in open source Intel GPU driver > and xfs. > > Talking about code size. You will be surprised if you really calculate it. > > For function amdgpu_cs_get_threshold_for_moves: > New code: It is 2 more loops and 2 more ifs. > Old code: 2 spinlock inline function. spin_lock can expand to 5 function > calls and one if. One function call contains 7 parameters. > spin_unlock can expand to 4 function calls. > > By the way, you can config Linux kernel to disable some spinlock macro > expansion. But I don't think people really do that. > > In function amdgpu_cs_report_moved_bytes > New code: zero > Old code: 2 spinlock inline function. > > In Total: > > New code: It is 2 more loops and 2 more ifs. Maybe there are one or two > other tiny things. > Old code: 4 spinlock inline function. They are expanded to 18 function > calls. Among them, two function calls each contain 7 parameters. > > Please think about it. Are you still sure that the new code make code size > bigger? Now what is the next problem of the new change?
The code size means the size of source code, not binary. The fewer lines of amdgpu code that we need to get the job done, the better. The next issue is the risk of breaking this already hard-to-test code. Your logic would be OK if there was a measurable benefit even with the silliest microbenchmark you can find (and even if it were as low as 0.2% improvement). Without that, I can't accept it. Sorry. Marek _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
