Re: API/ABIs: Re: [RFC PATCH v2 0/2] Add a new acquire resource to query vcpu statistics

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.03.2023 11:38, Matias Ezequiel Vara Larsen wrote:
> On Wed, Mar 08, 2023 at 03:16:05PM +0100, Jan Beulich wrote:
>> On 08.03.2023 12:54, Matias Ezequiel Vara Larsen wrote:
>>> On Tue, Mar 07, 2023 at 11:12:00AM +0100, Jan Beulich wrote:
>>>> On 06.03.2023 15:23, Matias Ezequiel Vara Larsen wrote:
>>>>> - Xen shall use the "stats_active" field to indicate what it is 
>>>>> producing. In
>>>>>   this field, reserved bits shall be 0. This shall allow us to agree on 
>>>>> the
>>>>> layout even when producer and consumer are compiled with different 
>>>>> headers.
>>>>
>>>> I wonder how well such a bitfield is going to scale. It provides for
>>>> only 32 (maybe 64) counters. Of course this may seem a lot right now,
>>>> but you never know how quickly something like this can grow. Plus
>>>> with ...
>>>>
>>>
>>> Would it make sense to define it like this?:
>>>
>>> struct vcpu_shmem_stats {
>>> #define STATS_A (1u << 0)
>>> ...
>>> #define VCPU_STATS_MAGIC 0xaabbccdd
>>>      uint32_t magic;
>>>      uint32_t offset;  // roundup(sizeof(vcpu_shmem_stats) + 
>>> sizeof(uint32_t) * nr_stats, cacheline_size)
>>>      uint32_t size;    // sizeof(vcpu_stats)
>>>      uint32_t stride;  // roundup(sizeof(vcpu_stats), cacheline_size)
>>>      uint32_t nr_stats; // size of stats_active in uint32_t
>>>      uint32_t stats_active[XEN_FLEX_ARRAY_DIM];
>>>      ...
>>> };
>>
> 
> The use of stats_active[] is meant to have a bitmap that could scale thus not
> limiting the number of counters in the vcpu_stat structure to 32 or 64. I 
> can't
> see other way to have an unlimited number of counters though.
> 
>> Possibly, but this would make it harder to use the interface. An alternative
>> might be to specify that an actual stats value set to ~0 marks an inactive
>> element. Since these are 64-bit counters, with today's and tomorrow's
>> computers we won't be at risk of a counter reaching a value of 2^^64-1, I
>> guess. And even if there was one where such a risk could not be excluded
>> (e.g. because of using pretty large increments), one would merely need to
>> make sure to saturate at 2^^64-2. Plus at such a point one would need to
>> consider anyway to switch to 128-bit fields, as neither saturated nor
>> wrapped values are of much use to consumers.
>>
> 
> If I understand well, this use-case is in case an element in the stats_active
> bitmap becomes inactive, i.e., it is set to "0" in stats_active[]. You are
> proposing to set to ~0 the actual stats value to mark an inactive element. I
> may be missing something here but would not be enough to set to "0" the
> corresponding stats_active[] bit? 

The suggestion was to eliminate the need for stats_active[].

Jan