* [PATCH] timers: limit heap size
@ 2019-04-09 13:01 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2019-04-09 13:01 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
First and foremost make timer_softirq_action() avoid growing the heap
if its new size can't be stored without truncation. 64k entries is a
lot, and I don't think we're at high risk of running into the issue,
but I think it's better to not allow for hard to debug problems to
occur in the first place.
Furthermore also adjust the code such the size/limit fields becoming
unsigned int would at least work from a mere sizing point of view. For
this also switch various uses of plain int to unsigned int.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/timer.c
+++ b/xen/common/timer.c
@@ -63,9 +63,9 @@ static struct heap_metadata *heap_metada
}
/* Sink down element @pos of @heap. */
-static void down_heap(struct timer **heap, int pos)
+static void down_heap(struct timer **heap, unsigned int pos)
{
- int sz = heap_metadata(heap)->size, nxt;
+ unsigned int sz = heap_metadata(heap)->size, nxt;
struct timer *t = heap[pos];
while ( (nxt = (pos << 1)) <= sz )
@@ -84,7 +84,7 @@ static void down_heap(struct timer **hea
}
/* Float element @pos up @heap. */
-static void up_heap(struct timer **heap, int pos)
+static void up_heap(struct timer **heap, unsigned int pos)
{
struct timer *t = heap[pos];
@@ -103,8 +103,8 @@ static void up_heap(struct timer **heap,
/* Delete @t from @heap. Return TRUE if new top of heap. */
static int remove_from_heap(struct timer **heap, struct timer *t)
{
- int sz = heap_metadata(heap)->size;
- int pos = t->heap_offset;
+ unsigned int sz = heap_metadata(heap)->size;
+ unsigned int pos = t->heap_offset;
if ( unlikely(pos == sz) )
{
@@ -130,7 +130,7 @@ static int remove_from_heap(struct timer
/* Add new entry @t to @heap. Return TRUE if new top of heap. */
static int add_to_heap(struct timer **heap, struct timer *t)
{
- int sz = heap_metadata(heap)->size;
+ unsigned int sz = heap_metadata(heap)->size;
/* Fail if the heap is full. */
if ( unlikely(sz == heap_metadata(heap)->limit) )
@@ -463,9 +463,14 @@ static void timer_softirq_action(void)
if ( unlikely(ts->list != NULL) )
{
/* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
- int old_limit = heap_metadata(heap)->limit;
- int new_limit = ((old_limit + 1) << 4) - 1;
- struct timer **newheap = xmalloc_array(struct timer *, new_limit + 1);
+ unsigned int old_limit = heap_metadata(heap)->limit;
+ unsigned int new_limit = ((old_limit + 1) << 4) - 1;
+ struct timer **newheap = NULL;
+
+ /* Don't grow the heap beyond what is representable in its metadata. */
+ if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
+ new_limit + 1 )
+ newheap = xmalloc_array(struct timer *, new_limit + 1);
if ( newheap != NULL )
{
spin_lock_irq(&ts->lock);
@@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke
struct timers *ts;
unsigned long flags;
s_time_t now = NOW();
- int i, j;
+ unsigned int i, j;
printk("Dumping timer queues:\n");
@@ -556,7 +561,7 @@ static void dump_timerq(unsigned char ke
spin_lock_irqsave(&ts->lock, flags);
for ( j = 1; j <= heap_metadata(ts->heap)->size; j++ )
dump_timer(ts->heap[j], now);
- for ( t = ts->list, j = 0; t != NULL; t = t->list_next, j++ )
+ for ( t = ts->list; t != NULL; t = t->list_next )
dump_timer(t, now);
spin_unlock_irqrestore(&ts->lock, flags);
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Xen-devel] [PATCH] timers: limit heap size
@ 2019-04-09 13:01 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2019-04-09 13:01 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
First and foremost make timer_softirq_action() avoid growing the heap
if its new size can't be stored without truncation. 64k entries is a
lot, and I don't think we're at high risk of running into the issue,
but I think it's better to not allow for hard to debug problems to
occur in the first place.
Furthermore also adjust the code such the size/limit fields becoming
unsigned int would at least work from a mere sizing point of view. For
this also switch various uses of plain int to unsigned int.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/timer.c
+++ b/xen/common/timer.c
@@ -63,9 +63,9 @@ static struct heap_metadata *heap_metada
}
/* Sink down element @pos of @heap. */
-static void down_heap(struct timer **heap, int pos)
+static void down_heap(struct timer **heap, unsigned int pos)
{
- int sz = heap_metadata(heap)->size, nxt;
+ unsigned int sz = heap_metadata(heap)->size, nxt;
struct timer *t = heap[pos];
while ( (nxt = (pos << 1)) <= sz )
@@ -84,7 +84,7 @@ static void down_heap(struct timer **hea
}
/* Float element @pos up @heap. */
-static void up_heap(struct timer **heap, int pos)
+static void up_heap(struct timer **heap, unsigned int pos)
{
struct timer *t = heap[pos];
@@ -103,8 +103,8 @@ static void up_heap(struct timer **heap,
/* Delete @t from @heap. Return TRUE if new top of heap. */
static int remove_from_heap(struct timer **heap, struct timer *t)
{
- int sz = heap_metadata(heap)->size;
- int pos = t->heap_offset;
+ unsigned int sz = heap_metadata(heap)->size;
+ unsigned int pos = t->heap_offset;
if ( unlikely(pos == sz) )
{
@@ -130,7 +130,7 @@ static int remove_from_heap(struct timer
/* Add new entry @t to @heap. Return TRUE if new top of heap. */
static int add_to_heap(struct timer **heap, struct timer *t)
{
- int sz = heap_metadata(heap)->size;
+ unsigned int sz = heap_metadata(heap)->size;
/* Fail if the heap is full. */
if ( unlikely(sz == heap_metadata(heap)->limit) )
@@ -463,9 +463,14 @@ static void timer_softirq_action(void)
if ( unlikely(ts->list != NULL) )
{
/* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
- int old_limit = heap_metadata(heap)->limit;
- int new_limit = ((old_limit + 1) << 4) - 1;
- struct timer **newheap = xmalloc_array(struct timer *, new_limit + 1);
+ unsigned int old_limit = heap_metadata(heap)->limit;
+ unsigned int new_limit = ((old_limit + 1) << 4) - 1;
+ struct timer **newheap = NULL;
+
+ /* Don't grow the heap beyond what is representable in its metadata. */
+ if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
+ new_limit + 1 )
+ newheap = xmalloc_array(struct timer *, new_limit + 1);
if ( newheap != NULL )
{
spin_lock_irq(&ts->lock);
@@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke
struct timers *ts;
unsigned long flags;
s_time_t now = NOW();
- int i, j;
+ unsigned int i, j;
printk("Dumping timer queues:\n");
@@ -556,7 +561,7 @@ static void dump_timerq(unsigned char ke
spin_lock_irqsave(&ts->lock, flags);
for ( j = 1; j <= heap_metadata(ts->heap)->size; j++ )
dump_timer(ts->heap[j], now);
- for ( t = ts->list, j = 0; t != NULL; t = t->list_next, j++ )
+ for ( t = ts->list; t != NULL; t = t->list_next )
dump_timer(t, now);
spin_unlock_irqrestore(&ts->lock, flags);
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] timers: limit heap size
@ 2019-04-09 13:38 ` Andrew Cooper
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2019-04-09 13:38 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson, Julien Grall
On 09/04/2019 14:01, Jan Beulich wrote:
> @@ -463,9 +463,14 @@ static void timer_softirq_action(void)
> if ( unlikely(ts->list != NULL) )
> {
> /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
> - int old_limit = heap_metadata(heap)->limit;
> - int new_limit = ((old_limit + 1) << 4) - 1;
> - struct timer **newheap = xmalloc_array(struct timer *, new_limit + 1);
> + unsigned int old_limit = heap_metadata(heap)->limit;
> + unsigned int new_limit = ((old_limit + 1) << 4) - 1;
> + struct timer **newheap = NULL;
> +
> + /* Don't grow the heap beyond what is representable in its metadata. */
> + if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
> + new_limit + 1 )
> + newheap = xmalloc_array(struct timer *, new_limit + 1);
It would probably be helpful to have a warn_once/print_once in the case
that we do hit the metadata limit
What is the new_limit + 1 for? Is it an overflow check?
Given a) that we're not moving from uint16_t while we have 32bit
hypervisor builds in use, and b) the calculation of new_limit will
truncate before getting into a position where this overflow check would
trip, I don't think it is helpful to retain.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] [PATCH] timers: limit heap size
@ 2019-04-09 13:38 ` Andrew Cooper
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2019-04-09 13:38 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson, Julien Grall
On 09/04/2019 14:01, Jan Beulich wrote:
> @@ -463,9 +463,14 @@ static void timer_softirq_action(void)
> if ( unlikely(ts->list != NULL) )
> {
> /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
> - int old_limit = heap_metadata(heap)->limit;
> - int new_limit = ((old_limit + 1) << 4) - 1;
> - struct timer **newheap = xmalloc_array(struct timer *, new_limit + 1);
> + unsigned int old_limit = heap_metadata(heap)->limit;
> + unsigned int new_limit = ((old_limit + 1) << 4) - 1;
> + struct timer **newheap = NULL;
> +
> + /* Don't grow the heap beyond what is representable in its metadata. */
> + if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
> + new_limit + 1 )
> + newheap = xmalloc_array(struct timer *, new_limit + 1);
It would probably be helpful to have a warn_once/print_once in the case
that we do hit the metadata limit
What is the new_limit + 1 for? Is it an overflow check?
Given a) that we're not moving from uint16_t while we have 32bit
hypervisor builds in use, and b) the calculation of new_limit will
truncate before getting into a position where this overflow check would
trip, I don't think it is helpful to retain.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] timers: limit heap size
@ 2019-04-09 14:18 ` Wei Liu
0 siblings, 0 replies; 12+ messages in thread
From: Wei Liu @ 2019-04-09 14:18 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, xen-devel
On Tue, Apr 09, 2019 at 07:01:55AM -0600, Jan Beulich wrote:
> @@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke
> struct timers *ts;
> unsigned long flags;
> s_time_t now = NOW();
> - int i, j;
> + unsigned int i, j;
A further possible improvement is to move j within the scope of
for_each_online_cpu {}.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] [PATCH] timers: limit heap size
@ 2019-04-09 14:18 ` Wei Liu
0 siblings, 0 replies; 12+ messages in thread
From: Wei Liu @ 2019-04-09 14:18 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, xen-devel
On Tue, Apr 09, 2019 at 07:01:55AM -0600, Jan Beulich wrote:
> @@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke
> struct timers *ts;
> unsigned long flags;
> s_time_t now = NOW();
> - int i, j;
> + unsigned int i, j;
A further possible improvement is to move j within the scope of
for_each_online_cpu {}.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] timers: limit heap size
@ 2019-04-09 15:08 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2019-04-09 15:08 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel
>>> On 09.04.19 at 16:18, <wei.liu2@citrix.com> wrote:
> On Tue, Apr 09, 2019 at 07:01:55AM -0600, Jan Beulich wrote:
>> @@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke
>> struct timers *ts;
>> unsigned long flags;
>> s_time_t now = NOW();
>> - int i, j;
>> + unsigned int i, j;
>
> A further possible improvement is to move j within the scope of
> for_each_online_cpu {}.
In fact I did consider this, but decided against: It's "just" a
debugging function, and j is not the only variable that would
better move into the more narrow scope. Moving them all is
beyond the scope of this patch, though.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] [PATCH] timers: limit heap size
@ 2019-04-09 15:08 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2019-04-09 15:08 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel
>>> On 09.04.19 at 16:18, <wei.liu2@citrix.com> wrote:
> On Tue, Apr 09, 2019 at 07:01:55AM -0600, Jan Beulich wrote:
>> @@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke
>> struct timers *ts;
>> unsigned long flags;
>> s_time_t now = NOW();
>> - int i, j;
>> + unsigned int i, j;
>
> A further possible improvement is to move j within the scope of
> for_each_online_cpu {}.
In fact I did consider this, but decided against: It's "just" a
debugging function, and j is not the only variable that would
better move into the more narrow scope. Moving them all is
beyond the scope of this patch, though.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] timers: limit heap size
@ 2019-04-09 15:17 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2019-04-09 15:17 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson, Julien Grall
>>> On 09.04.19 at 15:38, <andrew.cooper3@citrix.com> wrote:
> On 09/04/2019 14:01, Jan Beulich wrote:
>> @@ -463,9 +463,14 @@ static void timer_softirq_action(void)
>> if ( unlikely(ts->list != NULL) )
>> {
>> /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
>> - int old_limit = heap_metadata(heap)->limit;
>> - int new_limit = ((old_limit + 1) << 4) - 1;
>> - struct timer **newheap = xmalloc_array(struct timer *, new_limit +
> 1);
>> + unsigned int old_limit = heap_metadata(heap)->limit;
>> + unsigned int new_limit = ((old_limit + 1) << 4) - 1;
>> + struct timer **newheap = NULL;
>> +
>> + /* Don't grow the heap beyond what is representable in its metadata. */
>> + if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
>> + new_limit + 1 )
>> + newheap = xmalloc_array(struct timer *, new_limit + 1);
>
> It would probably be helpful to have a warn_once/print_once in the case
> that we do hit the metadata limit
I can do this, albeit the lack of the constructs you suggest will
make this a little ugly.
> What is the new_limit + 1 for? Is it an overflow check?
Kind of: It avoids the second argument to xmalloc_array() to
degenerate.
> Given a) that we're not moving from uint16_t while we have 32bit
> hypervisor builds in use, and b) the calculation of new_limit will
> truncate before getting into a position where this overflow check would
> trip, I don't think it is helpful to retain.
But that's what I'm (trying to) state(ing) in the description:
Making the size half a pointer's width is surely an option, which
would make it 32 bits on 64-bit builds (and result in better code
to be generated on x86). From a size/limit field width's
perspective the changes done here would be sufficient. The left
shifting in down_heap() would still need taking care of if we really
were to go that route.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] [PATCH] timers: limit heap size
@ 2019-04-09 15:17 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2019-04-09 15:17 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson, Julien Grall
>>> On 09.04.19 at 15:38, <andrew.cooper3@citrix.com> wrote:
> On 09/04/2019 14:01, Jan Beulich wrote:
>> @@ -463,9 +463,14 @@ static void timer_softirq_action(void)
>> if ( unlikely(ts->list != NULL) )
>> {
>> /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
>> - int old_limit = heap_metadata(heap)->limit;
>> - int new_limit = ((old_limit + 1) << 4) - 1;
>> - struct timer **newheap = xmalloc_array(struct timer *, new_limit +
> 1);
>> + unsigned int old_limit = heap_metadata(heap)->limit;
>> + unsigned int new_limit = ((old_limit + 1) << 4) - 1;
>> + struct timer **newheap = NULL;
>> +
>> + /* Don't grow the heap beyond what is representable in its metadata. */
>> + if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
>> + new_limit + 1 )
>> + newheap = xmalloc_array(struct timer *, new_limit + 1);
>
> It would probably be helpful to have a warn_once/print_once in the case
> that we do hit the metadata limit
I can do this, albeit the lack of the constructs you suggest will
make this a little ugly.
> What is the new_limit + 1 for? Is it an overflow check?
Kind of: It avoids the second argument to xmalloc_array() to
degenerate.
> Given a) that we're not moving from uint16_t while we have 32bit
> hypervisor builds in use, and b) the calculation of new_limit will
> truncate before getting into a position where this overflow check would
> trip, I don't think it is helpful to retain.
But that's what I'm (trying to) state(ing) in the description:
Making the size half a pointer's width is surely an option, which
would make it 32 bits on 64-bit builds (and result in better code
to be generated on x86). From a size/limit field width's
perspective the changes done here would be sufficient. The left
shifting in down_heap() would still need taking care of if we really
were to go that route.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] timers: limit heap size
@ 2019-04-09 16:22 ` Julien Grall
0 siblings, 0 replies; 12+ messages in thread
From: Julien Grall @ 2019-04-09 16:22 UTC (permalink / raw)
To: Jan Beulich, Andrew Cooper, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson
On 09/04/2019 16:17, Jan Beulich wrote:
>>>> On 09.04.19 at 15:38, <andrew.cooper3@citrix.com> wrote:
>> On 09/04/2019 14:01, Jan Beulich wrote:
>>> @@ -463,9 +463,14 @@ static void timer_softirq_action(void)
>>> if ( unlikely(ts->list != NULL) )
>>> {
>>> /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
>>> - int old_limit = heap_metadata(heap)->limit;
>>> - int new_limit = ((old_limit + 1) << 4) - 1;
>>> - struct timer **newheap = xmalloc_array(struct timer *, new_limit +
>> 1);
>>> + unsigned int old_limit = heap_metadata(heap)->limit;
>>> + unsigned int new_limit = ((old_limit + 1) << 4) - 1;
>>> + struct timer **newheap = NULL;
>>> +
>>> + /* Don't grow the heap beyond what is representable in its metadata. */
>>> + if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
>>> + new_limit + 1 )
>>> + newheap = xmalloc_array(struct timer *, new_limit + 1);
>>
>> It would probably be helpful to have a warn_once/print_once in the case
>> that we do hit the metadata limit
>
> I can do this, albeit the lack of the constructs you suggest will
> make this a little ugly.
Macros implementing such behavior would be more than welcomed. We tend to
open-code WARN_ONCE/PRINT_ONCE logic in every place which is not very nice.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] [PATCH] timers: limit heap size
@ 2019-04-09 16:22 ` Julien Grall
0 siblings, 0 replies; 12+ messages in thread
From: Julien Grall @ 2019-04-09 16:22 UTC (permalink / raw)
To: Jan Beulich, Andrew Cooper, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson
On 09/04/2019 16:17, Jan Beulich wrote:
>>>> On 09.04.19 at 15:38, <andrew.cooper3@citrix.com> wrote:
>> On 09/04/2019 14:01, Jan Beulich wrote:
>>> @@ -463,9 +463,14 @@ static void timer_softirq_action(void)
>>> if ( unlikely(ts->list != NULL) )
>>> {
>>> /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
>>> - int old_limit = heap_metadata(heap)->limit;
>>> - int new_limit = ((old_limit + 1) << 4) - 1;
>>> - struct timer **newheap = xmalloc_array(struct timer *, new_limit +
>> 1);
>>> + unsigned int old_limit = heap_metadata(heap)->limit;
>>> + unsigned int new_limit = ((old_limit + 1) << 4) - 1;
>>> + struct timer **newheap = NULL;
>>> +
>>> + /* Don't grow the heap beyond what is representable in its metadata. */
>>> + if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
>>> + new_limit + 1 )
>>> + newheap = xmalloc_array(struct timer *, new_limit + 1);
>>
>> It would probably be helpful to have a warn_once/print_once in the case
>> that we do hit the metadata limit
>
> I can do this, albeit the lack of the constructs you suggest will
> make this a little ugly.
Macros implementing such behavior would be more than welcomed. We tend to
open-code WARN_ONCE/PRINT_ONCE logic in every place which is not very nice.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-09 16:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09 13:01 [PATCH] timers: limit heap size Jan Beulich
2019-04-09 13:01 ` [Xen-devel] " Jan Beulich
2019-04-09 13:38 ` Andrew Cooper
2019-04-09 13:38 ` [Xen-devel] " Andrew Cooper
2019-04-09 15:17 ` Jan Beulich
2019-04-09 15:17 ` [Xen-devel] " Jan Beulich
2019-04-09 16:22 ` Julien Grall
2019-04-09 16:22 ` [Xen-devel] " Julien Grall
2019-04-09 14:18 ` Wei Liu
2019-04-09 14:18 ` [Xen-devel] " Wei Liu
2019-04-09 15:08 ` Jan Beulich
2019-04-09 15:08 ` [Xen-devel] " Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.