Re: [Qemu-devel] [Qemu-block] [PATCH 09/18] throttle-groups: protect throttled requests with a CoMutex

2017-05-29 Thread Alberto Garcia
On Thu 25 May 2017 06:32:16 PM CEST, Paolo Bonzini wrote:
> Another possibility is to use tg->lock, which we're holding anyway in
> both schedule_next_request and throttle_group_co_io_limits_intercept.
> This would require open-coding the CoQueue however, so I've chosen this
> alternative.
>
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Alberto Garcia 

Berto



Re: [Qemu-devel] [Qemu-block] [PATCH 09/18] throttle-groups: protect throttled requests with a CoMutex

2017-05-18 Thread Alberto Garcia
On Thu 18 May 2017 02:08:32 PM CEST, Paolo Bonzini wrote:
>> Does io_limits_disabled need locking too?
>
> It uses atomic access since patch 3.

Ah, I overlooked that one, sorry!

Berto



Re: [Qemu-devel] [Qemu-block] [PATCH 09/18] throttle-groups: protect throttled requests with a CoMutex

2017-05-18 Thread Paolo Bonzini


On 18/05/2017 14:06, Alberto Garcia wrote:
> On Thu 11 May 2017 04:41:59 PM CEST, Paolo Bonzini wrote:
> 
>> --- a/include/sysemu/block-backend.h
>> +++ b/include/sysemu/block-backend.h
>> @@ -72,11 +72,8 @@ typedef struct BlockDevOps {
>>   * fields that must be public. This is in particular for QLIST_ENTRY() and
>>   * friends so that BlockBackends can be kept in lists outside 
>> block-backend.c */
>>  typedef struct BlockBackendPublic {
>> -/* I/O throttling has its own locking, but also some fields are
>> - * protected by the AioContext lock.
>> - */
>> -
>> -/* Protected by AioContext lock.  */
>> +/* throttled_reqs_lock protects the CoQueues for throttled requests.  */
>> +CoMutex  throttled_reqs_lock;
>>  CoQueue  throttled_reqs[2];
>>  
>>  /* Nonzero if the I/O limits are currently being ignored; generally
>>   * it is zero.  */
>>  unsigned int io_limits_disabled;
> 
> Does io_limits_disabled need locking too?

It uses atomic access since patch 3.

Paolo



Re: [Qemu-devel] [Qemu-block] [PATCH 09/18] throttle-groups: protect throttled requests with a CoMutex

2017-05-18 Thread Alberto Garcia
On Thu 11 May 2017 04:41:59 PM CEST, Paolo Bonzini wrote:

> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -72,11 +72,8 @@ typedef struct BlockDevOps {
>   * fields that must be public. This is in particular for QLIST_ENTRY() and
>   * friends so that BlockBackends can be kept in lists outside 
> block-backend.c */
>  typedef struct BlockBackendPublic {
> -/* I/O throttling has its own locking, but also some fields are
> - * protected by the AioContext lock.
> - */
> -
> -/* Protected by AioContext lock.  */
> +/* throttled_reqs_lock protects the CoQueues for throttled requests.  */
> +CoMutex  throttled_reqs_lock;
>  CoQueue  throttled_reqs[2];
>  
>  /* Nonzero if the I/O limits are currently being ignored; generally
>   * it is zero.  */
>  unsigned int io_limits_disabled;

Does io_limits_disabled need locking too?

Berto