RFR: 8205051: UseNUMA memory interleaving vs cpunodebind & localalloc [v3]

[Swati Sharma]

On Wed, 27 Nov 2024 12:46:15 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Swati Sharma has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8205051: Added an extra method disable_numa to handle individual cases, minor comment change
>
> src/hotspot/os/linux/os_linux.cpp line 4502:
> 
>> 4500:         (Linux::is_running_in_interleave_mode() && Linux::_numa_interleave_bitmask != nullptr &&
>> 4501:         Linux::_numa_cpunodebind_bitmask != nullptr &&
>> 4502:         !_numa_bitmask_equal(Linux::_numa_interleave_bitmask, Linux::_numa_cpunodebind_bitmask))) {
> 
> Please extract this to a separate helper like the others. Maybe you can come up with a better name but something like: 
> Suggestion:
> 
>     if (Linux::numa_max_node() < 1 ||
>         Linux::is_bound_to_single_node() ||
>         Linux::mem_and_cpu_node_mismatch()) {
> 
> 
> Here is a suggestion on the helper to make it more readable, please make sure I got the logic right.
> ```  
> static bool mem_and_cpu_node_mismatch() {
>     struct bitmask* mem_nodes_bitmask = Linux::_numa_membind_bitmask;
>     if (Linux::is_running_in_interleave_mode()) {
>       mem_nodes_bitmask = Linux::_numa_interleave_bitmask;
>     }
> 
>     if (mem_nodes_bitmask == nullptr || Linux::_numa_cpunodebind_bitmask == nullptr) {
>       return false;
>     }
> 
>     return !_numa_bitmask_equal(mem_nodes_bitmask, Linux::_numa_cpunodebind_bitmask);
>   }

Done.

> src/hotspot/os/linux/os_linux.cpp line 4508:
> 
>> 4506:         warning("UseNUMA is disabled as the process bound to a single numa node"
>> 4507:                 " or cpu and memory nodes are not aligned");
>> 4508:       FLAG_SET_ERGO(UseNUMA, false);
> 
> I think we should check both `UseNUMA` and `UseNUMAInterleaving` here and set both accordingly. 
> Suggestion:
> 
>       // Disable NUMA support if:
>       // 1. Only a single NUMA node is available
>       // 2. The process is bound to a single NUMA node
>       // 3. The process memory and cpu node configuration is misaligned
>       if ((UseNUMA && FLAG_IS_CMDLINE(UseNUMA)) ||
>           (UseNUMAInterleaving && FLAG_IS_CMDLINE(UseNUMAInterleaving))) {
>         // Only issue a warning if the user explicitly asked for NUMA support
> 	      log_warning(os)("NUMA support is disabled as the process bound to a single"
> 		                    " numa node or cpu and memory nodes are not aligned");
>       }
>       FLAG_SET_ERGO(UseNUMA, false);
>       FLAG_SET_ERGO(UseNUMAInterleaving, false);
> 
> 
> I also changed to use `log_warning(os)` here instead of the old `warning`, but I see that we use the old warning below and we should probably make sure those are using the same. I would prefer `log_warning(os)` but this is not a strong request.

Addressed the change.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22395#discussion_r1883706151
PR Review Comment: https://git.openjdk.org/jdk/pull/22395#discussion_r1883708378