Re: [Xen-devel][Pv-ops][PATCH] Netback multiple tasklet support
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
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.
Xen-devel mailing list