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
0001-Netback-Generilize-static-global-variables-into-stru.patch
Description: 0001-Netback-Generilize-static-global-variables-into-stru.patch
0002-Netback-Multiple-tasklets-support.patch
Description: 0002-Netback-Multiple-tasklets-support.patch
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
|
Home