Re: [Qemu-devel] [PATCH 1/3] colo-compare: reconstruct the mutex lock usage

[Jason Wang <jasowang@redhat.com>]

On 2017年02月06日 16:13, Hailiang Zhang wrote:
> On 2017/2/3 11:47, Jason Wang wrote:
>>
>>
>> On 2017年01月24日 22:05, zhanghailiang wrote:
>>> The original 'timer_check_lock' mutex lock of struct CompareState
>>> is used to protect the 'conn_list' queue and its child queues which
>>> are 'primary_list' and 'secondary_list', which is a little abused
>>> and confusing
>>>
>>> To make it clearer, we rename 'timer_check_lock' to 'conn_list_lock'
>>> which is used to protect 'conn_list' queue, use another 'conn_lock'
>>> to protect 'primary_list' and 'secondary_list'.
>>>
>>> Besides, fix some missing places which need these mutex lock.
>>>
>>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com>
>>
>> Instead of sticking to such kind of mutex, I think it's time to make
>> colo timer run in colo thread (there's a TODO in the code).
>>
>
> Er, it seems that, we still need these mutex locks even we make colo
> timer and colo thread run in the same thread, because we may access
> the connect/primary/secondary list from colo (migratioin) thread
> concurrently.

Just make sure I understand the issue, why need it access the list?

>
> Besides, it seems to be a little complex to make colo timer run in colo
> compare thread, and it is not this series' goal. 

Seems not by just looking at how it was implemented in main loop, but 
maybe I was wrong.

> This series is preparing
> work for integrating COLO compare with COLO frame and it is prerequisite.
>
> So, we may consider implementing it later in another series.
> Zhang Chen, what's your opinion ?

The problem is this patch make things even worse, it introduces one more 
mutex.

Thanks

>
>
> Thanks,
> Hailiang
>
>> Thought?
>>
>> Thanks
>>
>> .
>>
>