RE: [Xen-devel][Pv-ops][PATCH] Netback multiple tasklet support

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

Jeremy, 
        I have revised the patch according to your suggestion. See attachment. 
0001: Keep group number as 1, and put all the global/static variables to struct 
xen_netbk. Do some preparations for multiple tasklets support.
0002: Support for netback multiple tasklet.
0003: Use kernel thread to replace the tasklet in order to ensure the dom0 
userspace QoS.

Thanks!
Dongxiao.

Jeremy Fitzhardinge wrote:
> On 12/03/09 18:13, Xu, Dongxiao wrote:
>>>      * same with "foreign_page_tracker"
>>>            o (the foreign page tracker API should have better names,
>>>              but that's not your problem)
>>>      * What's cpu_online_nr for?  I don't think it should be
>>>        necessary at all, and if it is, then it needs a much more
>>>      distinct name. * If they're really per-cpu variables, they
>>> should use the percpu        mechanism 
>>> 
>> Actually those tasklets are not per-cpu variables.
>> We just defined cpu_online_nr of tasklets, in order to get the best
>> performance if each tasklet could run on each cpu. However, they are
>> not binded with cpus. Some tasklets may run on the same vcpu of dom0
>> due to interrupt delivery affinity. Therefore these tasklets are not
>> per-cpu variables. 
>> 
> 
> OK, you should name the variable to what it actually means, not what
> its value happens to be.  It seems like a parameter which should be
> adjustable via sysfs or something.
> 
> How did you arrive at 3?
> 
>>>      * How do you relate the number of online CPUs to the whole
>>>        group index/pending index computation?  It isn't obvious how
>>>        they're connected, or how it guarantees that the index is
>>> enough. 
>>> 
>> Same explaination as above. Whether online cpus number is greater or
>> less than tasklet number does not matter in our case. We defined
>> them to the same value is only for getting best performance.
>> 
> 
> Nevertheless, it isn't at all clear how we can be certain the index
> calculations are less guaranteed to be less than the number of
> tasklets.  There is a lot of code scattered around the place; perhaps
> you could condense it into a smaller number of places?
> 
> In fact, the overall patch size is very large, and hard to review and
> test.  Could you please give some thought to how you can incrementally
> modify netback to get the result you want.  For example, keep the
> current functional structure, but make the changes to generalize to N
> processing handlers (but keeping N=1), then convert the softirq to a
> tasklet, then make N > 1.
> 
> Thanks,
>      J

Attachment: 0001-Netback-Generilize-static-global-variables-into-stru.patch
Description: 0001-Netback-Generilize-static-global-variables-into-stru.patch

Attachment: 0002-Netback-Multiple-tasklets-support.patch
Description: 0002-Netback-Multiple-tasklets-support.patch

Attachment: 0003-Use-Kernel-thread-to-replace-the-tasklet.patch
Description: 0003-Use-Kernel-thread-to-replace-the-tasklet.patch

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