[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.