[PATCH 3/3] block/nbd: nbd_co_reconnect_loop(): don't sleep if drained

Vladimir Sementsov-Ogievskiy posted 3 patches 5 years, 3 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Eric Blake <eblake@redhat.com>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
[PATCH 3/3] block/nbd: nbd_co_reconnect_loop(): don't sleep if drained
Posted by Vladimir Sementsov-Ogievskiy 5 years, 3 months ago
We try to go to wakeable sleep, so that, if drain begins it will break
the sleep. But what if nbd_client_co_drain_begin() already called and
s->drained is already true? We'll go to sleep, and drain will have to
wait for the whole timeout. Let's improve it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/nbd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 42146a26f7..d9cde30457 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -327,8 +327,6 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
             qemu_co_queue_restart_all(&s->free_sema);
         }
 
-        qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
-                                  &s->connection_co_sleep_ns_state);
         if (s->drained) {
             bdrv_dec_in_flight(s->bs);
             s->wait_drained_end = true;
@@ -340,9 +338,12 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
                 qemu_coroutine_yield();
             }
             bdrv_inc_in_flight(s->bs);
-        }
-        if (timeout < max_timeout) {
-            timeout *= 2;
+        } else {
+            qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
+                                      &s->connection_co_sleep_ns_state);
+            if (timeout < max_timeout) {
+                timeout *= 2;
+            }
         }
 
         nbd_reconnect_attempt(s);
-- 
2.21.0


Re: [PATCH 3/3] block/nbd: nbd_co_reconnect_loop(): don't sleep if drained
Posted by Eric Blake 5 years, 3 months ago
On 7/20/20 4:00 AM, Vladimir Sementsov-Ogievskiy wrote:
> We try to go to wakeable sleep, so that, if drain begins it will break
> the sleep. But what if nbd_client_co_drain_begin() already called and
> s->drained is already true? We'll go to sleep, and drain will have to
> wait for the whole timeout. Let's improve it.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/nbd.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 

How frequently did you hit this case?  At any rate, the optimization 
looks sane, and I'm happy to include it in 5.1.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH 3/3] block/nbd: nbd_co_reconnect_loop(): don't sleep if drained
Posted by Vladimir Sementsov-Ogievskiy 5 years, 3 months ago
23.07.2020 21:55, Eric Blake wrote:
> On 7/20/20 4:00 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We try to go to wakeable sleep, so that, if drain begins it will break
>> the sleep. But what if nbd_client_co_drain_begin() already called and
>> s->drained is already true? We'll go to sleep, and drain will have to
>> wait for the whole timeout. Let's improve it.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/nbd.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
> 
> How frequently did you hit this case?  At any rate, the optimization looks sane, and I'm happy to include it in 5.1.
> 

I don't remember. Probably once? And, as I said in cover letter, it even was not master branch but some my experiment..

-- 
Best regards,
Vladimir