[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 2/7] curl: never invoke callbacks with s->mutex
From: |
Jeff Cody |
Subject: |
Re: [Qemu-block] [PATCH 2/7] curl: never invoke callbacks with s->mutex held |
Date: |
Thu, 11 May 2017 16:40:03 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, May 10, 2017 at 04:32:00PM +0200, Paolo Bonzini wrote:
> All curl callbacks go through curl_multi_do, and hence are called with
> s->mutex held. Note that with comments, and make curl_read_cb drop the
> lock before invoking the callback.
>
> Likewise for curl_find_buf, where the callback can be invoked by the
> caller.
>
> Cc: address@hidden
> Cc: address@hidden
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> block/curl.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/block/curl.c b/block/curl.c
> index 25a301e7b4..9a00fdc28e 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -147,6 +147,7 @@ static void curl_multi_do(void *arg);
> static void curl_multi_read(void *arg);
>
> #ifdef NEED_CURL_TIMER_CALLBACK
> +/* Called from curl_multi_do_locked, with s->mutex held. */
> static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
> {
> BDRVCURLState *s = opaque;
> @@ -163,6 +164,7 @@ static int curl_timer_cb(CURLM *multi, long timeout_ms,
> void *opaque)
> }
> #endif
>
> +/* Called from curl_multi_do_locked, with s->mutex held. */
> static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
> void *userp, void *sp)
> {
> @@ -212,6 +214,7 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int
> action,
> return 0;
> }
>
> +/* Called from curl_multi_do_locked, with s->mutex held. */
> static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void
> *opaque)
> {
> BDRVCURLState *s = opaque;
> @@ -226,6 +229,7 @@ static size_t curl_header_cb(void *ptr, size_t size,
> size_t nmemb, void *opaque)
> return realsize;
> }
>
> +/* Called from curl_multi_do_locked, with s->mutex held. */
> static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void
> *opaque)
> {
> CURLState *s = ((CURLState*)opaque);
> @@ -264,7 +268,9 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t
> nmemb, void *opaque)
> request_length - offset);
> }
>
> + qemu_mutex_unlock(&s->s->mutex);
> acb->common.cb(acb->common.opaque, 0);
> + qemu_mutex_lock(&s->s->mutex);
> qemu_aio_unref(acb);
> s->acb[i] = NULL;
> }
> @@ -305,8 +311,6 @@ static int curl_find_buf(BDRVCURLState *s, size_t start,
> size_t len,
> if (clamped_len < len) {
> qemu_iovec_memset(acb->qiov, clamped_len, 0, len -
> clamped_len);
> }
> - acb->common.cb(acb->common.opaque, 0);
> -
> return FIND_RET_OK;
> }
>
> @@ -832,8 +836,8 @@ static void curl_readv_bh_cb(void *p)
> // we can just call the callback and be done.
> switch (curl_find_buf(s, start, acb->nb_sectors * BDRV_SECTOR_SIZE,
> acb)) {
> case FIND_RET_OK:
> - qemu_aio_unref(acb);
> - // fall through
> + ret = 0;
> + goto out;
> case FIND_RET_WAIT:
> goto out;
> default:
> --
> 2.12.2
>
>
Reviewed-by: Jeff Cody <address@hidden>
- [Qemu-block] [PATCH v2 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll, Paolo Bonzini, 2017/05/10
- [Qemu-block] [PATCH 1/7] curl: strengthen assertion in curl_clean_state, Paolo Bonzini, 2017/05/10
- [Qemu-block] [PATCH 2/7] curl: never invoke callbacks with s->mutex held, Paolo Bonzini, 2017/05/10
- [Qemu-block] [PATCH 3/7] curl: avoid recursive locking of BDRVCURLState mutex, Paolo Bonzini, 2017/05/10
- [Qemu-block] [PATCH 4/7] curl: split curl_find_state/curl_init_state, Paolo Bonzini, 2017/05/10
- [Qemu-block] [PATCH 5/7] curl: convert CURLAIOCB to byte values, Paolo Bonzini, 2017/05/10