WyvernTKC / cpuminer-gr-avx2

Optimised Version of GR miner for RTM
GNU General Public License v2.0
371 stars 193 forks source link

Affinity incorrectly set on > 63 thread CPUs (Mask roll-over due to wrong typ) #42

Closed Chipcraft closed 3 years ago

Chipcraft commented 3 years ago

It is still broken in the most recent commit. Currently you must (re)set the process affinity to make it work (i.e. open affinity and close it without making any changes).

You are unreachable in Discord, since you are not accepting friend requests.

Originally posted by @Chipcraft in https://github.com/WyvernTKC/cpuminer-gr-avx2/issues/41#issuecomment-940912442

Currently the mask is limited to 32-bit, causing > 32 threads to have the same mask as < 32 threads.

cpu-miner.c (2718): affine_to_cpu_mask(thr_id, 1 << (thr_id % num_cpus));

Change to:

affine_to_cpu_mask(thr_id, (unsigned long long int) 1 << (thr_id % num_cpus));

Fixes the issue, as it allows the mask being set properly (64-bit instead of 32-bit).

michal-zurkowski commented 3 years ago

Yes, I can see it. I'm also confirming with another person if it works ok on 24c/48t CPU. I'll put another binary up in the release with +32threads suffix to keep previous windows binaries untouched and 1:1 with released sources. Thanks for reporting on it. One last change that made affinity mast work correctly broke this up as I changed it to use a mask instead. I'll have to see how it fares on many other CPUs and configurations.

michal-zurkowski commented 3 years ago

Do you mind checking if those binaries work properly now for you too? If it is working correctly. Let me know so we can safely close the issue now (again :) )

Chipcraft commented 3 years ago

Do you mind checking if those binaries work properly now for you too? If it is working correctly. Let me know so we can safely close the issue now (again :) )

Binaries from "cpuminer-gr-1.2.1-x86_64_windows_32+threads" package are working fine and the performance is nominal, the issue is fixed. Feel free to close, thanks!

michal-zurkowski commented 3 years ago

Thank you again for reporting and final confirmation :)