From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Bennieston <andrew.bennieston@citrix.com>
Subject: Re: [PATCH RFC 1/4] xen-netback: Factor queue-specific
 data into queue struct.
Date: Thu, 16 Jan 2014 11:55:21 +0000
Message-ID: <52D7C8A9.5030609@citrix.com>
References: <1389803004-31812-1-git-send-email-andrew.bennieston@citrix.com>
	<1389803004-31812-2-git-send-email-andrew.bennieston@citrix.com>
	<20140116001706.GG5331@zion.uk.xensource.com>
	<52D7AC3C.6080101@citrix.com>
	<20140116113338.GS5698@zion.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <andrew.bennieston@citrix.com>) id 1W3lXi-000248-5E
	for xen-devel@lists.xenproject.org; Thu, 16 Jan 2014 11:55:26 +0000
In-Reply-To: <20140116113338.GS5698@zion.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Liu <wei.liu2@citrix.com>
Cc: xen-devel@lists.xenproject.org, paul.durrant@citrix.com, ian.campbell@citrix.com
List-Id: xen-devel@lists.xenproject.org

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