WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel][Pv-ops][PATCH 0/3 v3] Netback multiple threads support

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: RE: [Xen-devel][Pv-ops][PATCH 0/3 v3] Netback multiple threads support
From: "Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>
Date: Tue, 4 May 2010 19:36:58 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Steven Smith <steven.smith@xxxxxxxxxx>, KonradRzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Delivery-date: Tue, 04 May 2010 04:38:08 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4BE0073B02000078000011E7@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <D5AB6E638E5A3E4B8F4406B113A5A19A1D94B446@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <4BE0073B02000078000011E7@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcrrbXBcNkqSqY1hQUWYUuTp9HjXzQADiUYg
Thread-topic: [Xen-devel][Pv-ops][PATCH 0/3 v3] Netback multiple threads support
Jan Beulich wrote:
>>>> "Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx> 04.05.10 03:52 >>>
>> 1. Merge "group" and "idx" into "netif->mapping", therefore
>> page_ext is not used now.
> 
> Open coding this seems very fragile (and even more using literal
> constants in those code fragments).
> 
> I'm also not convinced restricting either part to 16 bits is a good
> thing (particularly on 64-bits, where you could easily have each part
> use 32 bits).

Do you have any suggestion on how to embed the data into
netif->mapping? 

> 
>> 2. Put netbk_add_netif() and netbk_remove_netif() into
>> __netif_up() and __netif_down().
> 
> An extra comment on top of what I said earlier: I don't think holding
> the lock for the entire duration of the loop in netbk_add_netif() is
> necessary - not doing so just makes the balancing slightly imprecise,
> but reduces (on large systems) the spin lock holding time (perhaps
> significantly).
> 
> If you don't, and if you instead make the netfront_count member
> an atomic_t, you can eliminate the lock altogether I would think.

I will modify it in next version of patch.

> 
>> 4. Use __get_free_pages() to replace kzalloc().
> 
> This is only marginally better than kzalloc(). Why not vmalloc(), as
> suggested?

OK.

> 
>> 6. Use MODPARM_netback_kthread to determine whether using
>> tasklet or kernel thread.
> 
> As a minor space optimization, the tasklet and kthread related
> fields in struct xen_netbk could be put in a union.

OK.

> 
>> 8. Add more checks in netif_page_release().
> 
> Actually, isn't
> 
>       BUG_ON(!PageForeign(page));
> 
> checking just a subset of
> 
>       BUG_ON(netbk->mmap_pages[idx] != page);
> 
> as all pages in those arrays would always be foreign?

Yes, you are right. That check is redundant and could be removed.

Thanks,
Dongxiao

> 
> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel