Re: [Xen-devel] [PATCH] xen/timers: Document and improve the representation of the timer heap metadata

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/03/2019 16:23, Jan Beulich wrote:
>>>> On 29.03.19 at 17:02, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/common/timer.c
>> +++ b/xen/common/timer.c
>> @@ -45,18 +45,27 @@ DEFINE_PER_CPU(s_time_t, timer_deadline);
>>  
>>  
>> /****************************************************************************
>>   * HEAP OPERATIONS.
>> + *
>> + * Slot 0 of the heap is never a valid timer pointer, and instead holds the
>> + * heap metadata.
>>   */
>>  
>> -#define GET_HEAP_SIZE(_h)     ((int)(((u16 *)(_h))[0]))
>> -#define SET_HEAP_SIZE(_h,_v)  (((u16 *)(_h))[0] = (u16)(_v))
>> +struct heap_metadata {
>> +    uint16_t size, limit;
>> +};
>> +
>> +static struct heap_metadata *heap_metadata(struct timer **heap)
>> +{
>> +    /* Check that our type-punning doesn't overflow into heap[1] */
>> +    BUILD_BUG_ON(sizeof(struct heap_metadata) > sizeof(struct timer *));
>>  
>> -#define GET_HEAP_LIMIT(_h)    ((int)(((u16 *)(_h))[1]))
>> -#define SET_HEAP_LIMIT(_h,_v) (((u16 *)(_h))[1] = (u16)(_v))
>> +    return (struct heap_metadata *)&heap[0];
>> +}
>>  
>>  /* Sink down element @pos of @heap. */
>>  static void down_heap(struct timer **heap, int pos)
>>  {
>> -    int sz = GET_HEAP_SIZE(heap), nxt;
>> +    int sz = heap_metadata(heap)->size, nxt;
> While I realize that it'll alter generated code, I think this would be
> a very good opportunity to convert various local variables to
> unsigned int. But I won't insist, so with or without the extra change
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

I know you were going to ask, but I'm not sufficiently confident with
being able to do that without error.  That is why I specifically called
out the check of no difference in the compiled binary.

Other changes are likely to happen at a future date.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel