>>> Steven Smith <steven.smith@xxxxxxxxxx> 30.04.10 09:29 >>>
>It sounds like you've had a pretty good look at these patches. Did
>you see anything else worth pointing out?
No, the major ones you had already pointed out.
Despite your earlier comments, even in the latest version the load
balancing code still doesn't seem right. I found it necessary to do
this right in __netif_{up,down}(), so that there is no potential of
mis-balanced increments and decrements (we also still have the
[pointless] list inserts/removes in our version of the patches,
which was what actually pointed out the issue by way of seeing
crashes or endless loops). This in turn made it necessary to add
logic to ignore the first (couple of?) invocation(s) of netif_be_int()
(i.e. before the netif as assigned to a group).
Also in spite of your earlier comments, the use of
kthread_should_stop() in the latest version of the patches still
seems insufficient - afaict it should also be used in the
expression passed to wait_event_interruptible().
Minor ones are
- should not use kzalloc() for allocating the (huge) array of struct
 xen_netbk in netback_init()
- the changes to netif_be_dbg() are bogus
- tasklets get replaced unconditionally with kthreads - I opted to
 make this dependent on a command line option, as the general
 description hinted both having their up- and downsides
- placement of fields in struct xen_netbk (I'd prefer all small fields to
 precede the large arrays - on x86 this results in smaller code)
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel