Re: [PATCH v16 08/14] numa: Extend CLI to provide memory latency and bandwidth information

[Tao Xu <tao3.xu@intel.com>]

On 11/19/2019 7:03 PM, Igor Mammedov wrote:
> On Fri, 15 Nov 2019 15:53:46 +0800
> Tao Xu <tao3.xu@intel.com> wrote:
> 
>> From: Liu Jingqi <jingqi.liu@intel.com>
>>
>> Add -numa hmat-lb option to provide System Locality Latency and
>> Bandwidth Information. These memory attributes help to build
>> System Locality Latency and Bandwidth Information Structure(s)
>> in ACPI Heterogeneous Memory Attribute Table (HMAT).
>>
>> Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
>> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> 
> looks good to me, so
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 
> 
> PS:
> also see question below
> 
[...]
>> +
>> +        hmat_lb->range_bitmap |= node->bandwidth;
>> +        first_bit = ctz64(hmat_lb->range_bitmap);
>> +        hmat_lb->base = UINT64_C(1) << first_bit;
>> +        max_entry = node->bandwidth / hmat_lb->base;
>> +        last_bit = 64 - clz64(hmat_lb->range_bitmap);
>> +
>> +        /*
>> +         * For bandwidth, first_bit record the base unit of bandwidth bits,
>> +         * last_bit record the last bit of the max bandwidth. The max compressed
>> +         * bandwidth should be less than 0xFFFF (UINT16_MAX)
>> +         */
>> +        if ((last_bit - first_bit) > UINT16_BITS || max_entry >= UINT16_MAX) {
>                                                         ^^^^^^^^^^^^^^^^^^^
> what bandwidth combination is going to trigger above condition?
> 
Only use (last_bit - first_bit) > UINT16_BITS, we can't trigger error if 
the max compressed bandwidth is 0xFFFF. Because in that condition, 
"last_bit - first_bit == UINT16_BITS". So I add "max_entry >= 
UINT16_MAX" to catch 0xFFFF. For example:

Combination 1 (Error):
bandwidth1   = ...0000 1111 1111 1111 1110 0000... (max_entry 32767)
range_bitmap = ...0000 1111 1111 1111 1110 0000... (range is 15 bits)
bandwidth2   = ...0000 1111 1111 1111 1111 0000... (max_entry 65535)
range_bitmap = ...0000 1111 1111 1111 1111 0000... (range is 16 bits)

Combination 2 (Error):
bandwidth1   = ...0000 1111 1111 1111 1110 0000... (max_entry 32767)
range_bitmap = ...0000 1111 1111 1111 1110 0000... (range is 15 bits)
bandwidth2   = ...0001 1111 1111 1111 1110 0000... (max_entry 65535)
range_bitmap = ...0001 1111 1111 1111 1110 0000... (range is 16 bits)

Combination 3 (OK, because bandwidth1 will be compressed to 65534):
bandwidth1   = ...0000 1111 1111 1111 1110 0000... (max_entry 32767)
range_bitmap = ...0000 1111 1111 1111 1110 0000... (range is 15 bits)
bandwidth2   = ...0000 0111 1111 1111 1111 0000... (max_entry 32767)
range_bitmap = ...0000 1111 1111 1111 1111 0000... (range is 16 bits)

Combination 4 (Error):
bandwidth1   = ...0000 1111 1111 1111 1111 0000... (max_entry 65535)
range_bitmap = ...0000 1111 1111 1111 1111 0000... (range is 16 bits)