On 10/28/2018 03:50 PM, Paolo Bonzini wrote:
On 27/10/2018 01:33, Emilio G. Cota wrote:
On Wed, Oct 17, 2018 at 12:10:15 +0200, Paolo Bonzini wrote:
On 16/10/2018 13:10, guangrong.xiao@xxxxxxxxx wrote:
An idea: the total number of requests is going to be very small, and a
PtrRing is not the nicest data structure for multiple producer/single
consumer. So you could instead:
(snip)
- now that you have request indices, you can replace the completion
ptr_ring with a bitmap, and set a bit in the bitmap with set_bit_atomic
to report completion. On the writer side you use find_next_bit to find
(snip)
Emilio, can you review the above ideas?
Sorry it took me a while to go through this.
I like your suggestions. Just one nit; I'm not sure I understood
the use case very well, but I think using a bitmap to signal
completion might be suboptimal, since we'd have several
thread spinning on the same cacheline yet caring about
different bits.
Requests are asynchronous, the bitmap is only used to find a free
submission slot. You're right that the bitmap can bounce across
processors, but I'm not sure how else you would do that because you
don't know in advance how many submitting threads you have. It wouldn't
be any worse if there was a spinlock.
However, in the migration case there is only one submitting thread, so
it's okay. :)
Yup.
The cache contention only exists in the work threads, the sumbiter thread
is totally free who is the main migration thread. Making the main thread
be faster is good.
Paolo
Xiao: a couple of suggestions
- Since you'll be adding a generic module, make its commit and
description self-contained. That is, mentioning in the
log that this will be used for migration is fine, but please
describe the module (and the assumptions it makes about its
users) in general, so that someone that doesn't know anything
about migration can still understand this module (and hopefully
adopt it for other use cases).
Good to me, i will add more detailed description for this module in
the next version.
- I'd like to see a simple test program (or rather, benchmark)
that shows how this works. This benchmark would be completely
unrelated to migration; it should just be a simple test of
the performance/scalability of this module.
Having this benchmark would help (1) discuss and quantitately
evaluate modifications to the module, and (2) help others to
quickly understand what the module does.
See tests/qht-bench.c for an example.
Can not agree with you more, will do. :)
Thank you, Emilio and Paolo!