Re: [Xen-devel] [PATCH RFC 1/4] xen-netback: Factor queue-specific data into queue struct.

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Jan 16, 2014 at 09:54:04AM +0000, Andrew Bennieston wrote:
> On 16/01/14 00:17, Wei Liu wrote:
> >On Wed, Jan 15, 2014 at 04:23:21PM +0000, Andrew J. Bennieston wrote:
> >[...]
> >>+
> >>+struct xenvif_queue { /* Per-queue data for xenvif */
> >>+   unsigned int number; /* Queue number, 0-based */
> >
> >Use "id" instead?
> 
> Ok; I suppose number implies "the number of queues", not "which queue is
> this?"
> 

Right. "id" sounds more intuitive to me. And it saves you from some
typing, big win! ;-)

> >
> >>+   char name[IFNAMSIZ+4]; /* DEVNAME-qN */
> >>+   struct xenvif *vif; /* Parent VIF */
[...]
> >>
> >>@@ -150,14 +152,27 @@ struct xenvif {
> >>     */
> >>    RING_IDX rx_req_cons_peek;
> >>
> >>-   /* This array is allocated seperately as it is large */
> >>-   struct gnttab_copy *grant_copy_op;
> >>+   struct gnttab_copy grant_copy_op[MAX_GRANT_COPY_OPS];
> >
> >Any reason to swtich back to array inside structure? This array is
> >really large.
> >
> 
> It was moved to a separate vmalloc because it was large, but now the
> array of queues is allocated through vmalloc anyway. I preferred to
> bring this back into the structure rather than have more allocations to
> track and remember to free at all relevant points. If there is any
> significant reason to split this out I'm happy to do so...
> 

OK, if everything is allocated via vmalloc then I think it's fine.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel