Re: [Xen-devel] [PATCH v5 2/2] xen/arm: consolidate make_timer_node and make_timer_domU_node

[Viktor Mitin <viktor.mitin.19@xxxxxxxxx>]

On Thu, Aug 1, 2019 at 4:58 PM Julien Grall <julien.grall@xxxxxxx> wrote:
>
>
>
> On 01/08/2019 14:57, Julien Grall wrote:

> >>> +    unsigned int irq[MAX_TIMER_PPI];
> >> As I said in the previous version, you are wasting stack space
> >> there. Also, this is misleading. When I see array of 4 items, I expect
> >> that all 4 items will be used. But you are using only 3, so it leads to
> >> two conclusions: either you made a mistake, or I don't understand what
> >> it happening. Either option is bad.
> >
> > 4 byte on a stack of 16KB... that's not really a waste worth an argument. 
> > The
> > more the stack is pretty empty as this is boot. So yes, you will not use the
> > last index because you don't expose hypervisor timer to guest yet! (Imagine
> > nested virt). But at least it avoids hardcoding a number of index and match 
> > the
> > enum.
>
> I forgot to mention. @Viktor, it is good to try to reply to each comment at
> least those you don't plan to address. So the reviewer doesn't have the 
> feeling
> comments are ignored...

Well, I address each of the comments or write about it explicitly in
other cases.
In this particular case, I'd added  '-1', but later did not merge it
due to mistake.
So it supposed to be the next:
+    unsigned int irq[MAX_TIMER_PPI-1]

There are no comments ignored from my side, at least not intentionally.
In this case, there were myriads of the small things, like add space
here or remove empty line there.
I did not agree with all of them, however, in the next version (in
v5), all of them have been addressed.

Thanks

> Cheers,
>
> --
> Julien Grall

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