Re: [PATCH v2 17/24] selftests/resctrl: Replace count_bits with count_consecutive_bits()

[Reinette Chatre]

Hi Ilpo,

On 4/25/2023 4:41 AM, Ilpo Järvinen wrote:
> On Fri, 21 Apr 2023, Reinette Chatre wrote:
>> On 4/18/2023 4:44 AM, Ilpo Järvinen wrote:
>>> CAT and CMT tests depends on masks being continuous.
>>
>> The term used in the spec is "contiguous", using it here
>> may help to convey the goal.
>>
>>>
>>> Replace count_bits with more appropriate variant that counts
>>> consecutive bits.
>>
>> Could you please elaborate why this is more appropriate and
>> why this is necessary? What is wrong with current solution?
>>
>> Please note that cbm_mask in resctrl will always be contiguous.
> 
> Hi,
> 
> It's good that you asked this question.
> 
> This comes from a change (not by me originally) that also excluded the 
> shareable bits from the mask the CAT test uses. I assumed (w/o better 
> knowledge) that those shareable bits could create a hole into the mask.

You are correct. Shareable bits can create a hole in the mask.

> 
> It could be wrong assumption and the shareable bits are always at one end 
> of the CBM mask.
> 
> Do you happen to know how the shareable bits are positioned within the 
> mask?

This depends on the hardware. Software learns about it via a bitmask (as
opposed to "number of bits") so the hardware has flexibility to communicate
any combination of shared ways.

These comments are not related to this patch though. I understand that
this patch has been created in support of the changes that follow. My questions
are related to this patch that communicates that it is "more appropriate"
for what the code currently does without consideration of what is to come.
I would like to understand how this is more appropriate. Also note
that cbm_mask will always be contiguous - in this case the hardware
communicates a number of bits, not a bitmask, so this will always be
contiguous. This patch claims that the current solution is not
appropriate to parse cbm_mask, could you please elaborate why?

Reinette