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

[Andrew Bennieston <andrew.bennieston@citrix.com>]

On 16/01/14 11:33, Wei Liu wrote:
> 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! ;-)

Actually it's more typing, since I've already typed "number" (and I 
don't think :%s/number/id/g is a safe thing to do!)... but I'll change 
it anyway. :)

Andrew

>
>>>
>>>> +	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.
>