Re: [PATCH 2/4] migration: introduce lockless multithreads model

[Paolo Bonzini <pbonzini@redhat.com>]

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@gmail.com 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. :)

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).
> 
> - 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.
> 
> Thanks,
> 
> 		Emilio
>