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

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

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

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

>4. Use __get_free_pages() to replace kzalloc().

This is only marginally better than kzalloc(). Why not vmalloc(), as
suggested?

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

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

Jan


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