[PATCH for-7.1 0/8] nbd: actually make s->state thread-safe

[PATCH for-7.1 0/8] nbd: actually make s->state thread-safe

[Paolo Bonzini]

The main point of this series is patch 6, which removes the dubious and
probably wrong use of atomics in block/nbd.c.  This in turn is enabled
mostly by the cleanups in patches 3-5.  Together, they introduce a
QemuMutex that synchronizes the NBD client coroutines, the reconnect_delay
timer and nbd_cancel_in_flight() as well.

The fixes happen to remove an incorrect use of qemu_co_queue_restart_all
and qemu_co_enter_next on the s->free_sema CoQueue, which was not guarded
by s->send_mutex.

The rest is bugfixes, simplifying the code a bit, and extra documentation.

Paolo Bonzini (8):
  nbd: actually implement reply_possible safeguard
  nbd: mark more coroutine_fns
  nbd: remove peppering of nbd_client_connected
  nbd: keep send_mutex/free_sema handling outside
    nbd_co_do_establish_connection
  nbd: use a QemuMutex to synchronize reconnection with coroutines
  nbd: move s->state under requests_lock
  nbd: take receive_mutex when reading requests[].receiving
  nbd: document what is protected by the CoMutexes

 block/coroutines.h |   4 +-
 block/nbd.c        | 303 +++++++++++++++++++++++----------------------
 2 files changed, 157 insertions(+), 150 deletions(-)

-- 
2.35.1

Re: [PATCH for-7.1 0/8] nbd: actually make s->state thread-safe

[Vladimir Sementsov-Ogievskiy]

12.04.2022 20:32, Paolo Bonzini wrote:
> The main point of this series is patch 6, which removes the dubious and
> probably wrong use of atomics in block/nbd.c.  This in turn is enabled
> mostly by the cleanups in patches 3-5.  Together, they introduce a
> QemuMutex that synchronizes the NBD client coroutines, the reconnect_delay
> timer and nbd_cancel_in_flight() as well.
> 
> The fixes happen to remove an incorrect use of qemu_co_queue_restart_all
> and qemu_co_enter_next on the s->free_sema CoQueue, which was not guarded
> by s->send_mutex.
> 
> The rest is bugfixes, simplifying the code a bit, and extra documentation.
> 
> Paolo Bonzini (8):
>    nbd: actually implement reply_possible safeguard
>    nbd: mark more coroutine_fns
>    nbd: remove peppering of nbd_client_connected
>    nbd: keep send_mutex/free_sema handling outside
>      nbd_co_do_establish_connection
>    nbd: use a QemuMutex to synchronize reconnection with coroutines
>    nbd: move s->state under requests_lock
>    nbd: take receive_mutex when reading requests[].receiving
>    nbd: document what is protected by the CoMutexes
> 
>   block/coroutines.h |   4 +-
>   block/nbd.c        | 303 +++++++++++++++++++++++----------------------
>   2 files changed, 157 insertions(+), 150 deletions(-)
> 

Hmm, no patches come to me except for cover-letter. Neither here: https://patchew.org/QEMU/20220412173216.308065-1-pbonzini@redhat.com/

-- 
Best regards,
Vladimir

Re: [PATCH for-7.1 0/8] nbd: actually make s->state thread-safe

[Paolo Bonzini]

On 4/12/22 20:26, Vladimir Sementsov-Ogievskiy wrote:
> 12.04.2022 20:32, Paolo Bonzini wrote:
>> The main point of this series is patch 6, which removes the dubious and
>> probably wrong use of atomics in block/nbd.c.  This in turn is enabled
>> mostly by the cleanups in patches 3-5.  Together, they introduce a
>> QemuMutex that synchronizes the NBD client coroutines, the 
>> reconnect_delay timer and nbd_cancel_in_flight() as well.
>>
>> The fixes happen to remove an incorrect use of qemu_co_queue_restart_all
>> and qemu_co_enter_next on the s->free_sema CoQueue, which was not guarded
>> by s->send_mutex.
>>
>> The rest is bugfixes, simplifying the code a bit, and extra 
>> documentation.
>>
>> Paolo Bonzini (8):
>>    nbd: actually implement reply_possible safeguard
>>    nbd: mark more coroutine_fns
>>    nbd: remove peppering of nbd_client_connected
>>    nbd: keep send_mutex/free_sema handling outside
>>      nbd_co_do_establish_connection
>>    nbd: use a QemuMutex to synchronize reconnection with coroutines
>>    nbd: move s->state under requests_lock
>>    nbd: take receive_mutex when reading requests[].receiving
>>    nbd: document what is protected by the CoMutexes
>>
>>   block/coroutines.h |   4 +-
>>   block/nbd.c        | 303 +++++++++++++++++++++++----------------------
>>   2 files changed, 157 insertions(+), 150 deletions(-)
>>
> 
> Hmm, no patches come to me except for cover-letter. Neither here: 
> https://patchew.org/QEMU/20220412173216.308065-1-pbonzini@redhat.com/

Network issue, I resent it a couple hours later (and forgot to Cc you, 
sorry about that).

Paolo

[PATCH for-7.1 0/8] nbd: actually make s->state thread-safe

[Paolo Bonzini]

The main point of this series is patch 6, which removes the dubious and
probably wrong use of atomics in block/nbd.c.  This in turn is enabled
mostly by the cleanups in patches 3-5.  Together, they introduce a
QemuMutex that synchronizes the NBD client coroutines, the reconnect_delay
timer and nbd_cancel_in_flight() as well.

The fixes happen to remove an incorrect use of qemu_co_queue_restart_all
and qemu_co_enter_next on the s->free_sema CoQueue, which was not guarded
by s->send_mutex.

The rest is bugfixes, simplifying the code a bit, and extra documentation.

Paolo Bonzini (8):
  nbd: actually implement reply_possible safeguard
  nbd: mark more coroutine_fns
  nbd: remove peppering of nbd_client_connected
  nbd: keep send_mutex/free_sema handling outside
    nbd_co_do_establish_connection
  nbd: use a QemuMutex to synchronize reconnection with coroutines
  nbd: move s->state under requests_lock
  nbd: take receive_mutex when reading requests[].receiving
  nbd: document what is protected by the CoMutexes

 block/coroutines.h |   4 +-
 block/nbd.c        | 295 +++++++++++++++++++++++----------------------
 2 files changed, 154 insertions(+), 145 deletions(-)

-- 
2.35.1