From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40533) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPYz9-0001hR-73 for qemu-devel@nongnu.org; Thu, 04 Sep 2014 11:30:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XPYz2-0007sS-PN for qemu-devel@nongnu.org; Thu, 04 Sep 2014 11:30:07 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:47328 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPYz2-0007s5-Eg for qemu-devel@nongnu.org; Thu, 04 Sep 2014 11:30:00 -0400 Date: Thu, 4 Sep 2014 17:29:13 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140904152913.GD8094@irqsave.net> References: <1409743435-21155-1-git-send-email-famz@redhat.com> <1409743435-21155-6-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1409743435-21155-6-git-send-email-famz@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 05/20] thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , Chrysostomos Nanakos , Ronnie Sahlberg , Peter Lieven , qemu-devel@nongnu.org, Paolo Bonzini , Stefan Hajnoczi , Josh Durgin , Liu Yuan , MORITA Kazutaka The Wednesday 03 Sep 2014 =E0 19:23:40 (+0800), Fam Zheng wrote : > The .cancel_async has the same the first half with .cancel: try to stea= l "The .cancel_async share the same first half with .cancel" ? > the request if not submitted yet. In this case set the elem to > THREAD_DONE status and ret to -ECANCELED, which means > thread_pool_completion_bh will call the cb with -ECANCELED. >=20 > If the request is already submitted, do nothing, as we know the normal > completion will happen in the future. >=20 > Testing code update: >=20 > Before, done_cb is only called if the request is already submitted by > thread pool. Now done_cb is always called, even before it is submitted, > because we emulate bdrv_aio_cancel with bdrv_aio_cancel_async. So also > update the test criteria accordingly. >=20 > Signed-off-by: Fam Zheng > --- > tests/test-thread-pool.c | 34 ++++++++++++++++++++++++++-------- > thread-pool.c | 32 ++++++++++++++------------------ > 2 files changed, 40 insertions(+), 26 deletions(-) >=20 > diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c > index f40b7fc..ed2b25b 100644 > --- a/tests/test-thread-pool.c > +++ b/tests/test-thread-pool.c > @@ -33,7 +33,7 @@ static int long_cb(void *opaque) > static void done_cb(void *opaque, int ret) > { > WorkerTestData *data =3D opaque; > - g_assert_cmpint(data->ret, =3D=3D, -EINPROGRESS); > + g_assert(data->ret =3D=3D -EINPROGRESS || data->ret =3D=3D -ECANCE= LED); > data->ret =3D ret; > data->aiocb =3D NULL; > =20 > @@ -132,7 +132,7 @@ static void test_submit_many(void) > } > } > =20 > -static void test_cancel(void) > +static void do_test_cancel(bool sync) > { > WorkerTestData data[100]; > int num_canceled; > @@ -170,18 +170,25 @@ static void test_cancel(void) > for (i =3D 0; i < 100; i++) { > if (atomic_cmpxchg(&data[i].n, 0, 3) =3D=3D 0) { > data[i].ret =3D -ECANCELED; > - bdrv_aio_cancel(data[i].aiocb); > - active--; > + if (sync) { > + bdrv_aio_cancel(data[i].aiocb); > + } else { > + bdrv_aio_cancel_async(data[i].aiocb); > + } > num_canceled++; > } > } > g_assert_cmpint(active, >, 0); > g_assert_cmpint(num_canceled, <, 100); > =20 > - /* Canceling the others will be a blocking operation. */ > for (i =3D 0; i < 100; i++) { > if (data[i].aiocb && data[i].n !=3D 3) { > - bdrv_aio_cancel(data[i].aiocb); > + if (sync) { > + /* Canceling the others will be a blocking operation. = */ > + bdrv_aio_cancel(data[i].aiocb); > + } else { > + bdrv_aio_cancel_async(data[i].aiocb); > + } > } > } > =20 > @@ -193,15 +200,25 @@ static void test_cancel(void) > for (i =3D 0; i < 100; i++) { > if (data[i].n =3D=3D 3) { > g_assert_cmpint(data[i].ret, =3D=3D, -ECANCELED); > - g_assert(data[i].aiocb !=3D NULL); > + g_assert(data[i].aiocb =3D=3D NULL); > } else { > g_assert_cmpint(data[i].n, =3D=3D, 2); > - g_assert_cmpint(data[i].ret, =3D=3D, 0); > + g_assert(data[i].ret =3D=3D 0 || data[i].ret =3D=3D -ECANC= ELED); > g_assert(data[i].aiocb =3D=3D NULL); > } > } > } > =20 > +static void test_cancel(void) > +{ > + do_test_cancel(true); > +} > + > +static void test_cancel_async(void) > +{ > + do_test_cancel(false); > +} > + > int main(int argc, char **argv) > { > int ret; > @@ -217,6 +234,7 @@ int main(int argc, char **argv) > g_test_add_func("/thread-pool/submit-co", test_submit_co); > g_test_add_func("/thread-pool/submit-many", test_submit_many); > g_test_add_func("/thread-pool/cancel", test_cancel); > + g_test_add_func("/thread-pool/cancel-async", test_cancel_async); > =20 > ret =3D g_test_run(); > =20 > diff --git a/thread-pool.c b/thread-pool.c > index 23888dc..6afd343 100644 > --- a/thread-pool.c > +++ b/thread-pool.c > @@ -32,7 +32,6 @@ enum ThreadState { > THREAD_QUEUED, > THREAD_ACTIVE, > THREAD_DONE, > - THREAD_CANCELED, > }; > =20 > struct ThreadPoolElement { > @@ -59,7 +58,6 @@ struct ThreadPool { > AioContext *ctx; > QEMUBH *completion_bh; > QemuMutex lock; > - QemuCond check_cancel; > QemuCond worker_stopped; > QemuSemaphore sem; > int max_threads; > @@ -74,7 +72,6 @@ struct ThreadPool { > int idle_threads; > int new_threads; /* backlog of threads we need to create */ > int pending_threads; /* threads created but not running yet */ > - int pending_cancellations; /* whether we need a cond_broadcast */ > bool stopping; > }; > =20 > @@ -114,9 +111,6 @@ static void *worker_thread(void *opaque) > req->state =3D THREAD_DONE; > =20 > qemu_mutex_lock(&pool->lock); > - if (pool->pending_cancellations) { > - qemu_cond_broadcast(&pool->check_cancel); > - } > =20 > qemu_bh_schedule(pool->completion_bh); > } > @@ -174,7 +168,7 @@ static void thread_pool_completion_bh(void *opaque) > =20 > restart: > QLIST_FOREACH_SAFE(elem, &pool->head, all, next) { > - if (elem->state !=3D THREAD_CANCELED && elem->state !=3D THREA= D_DONE) { > + if (elem->state !=3D THREAD_DONE) { > continue; > } > if (elem->state =3D=3D THREAD_DONE) { > @@ -218,22 +212,26 @@ static void thread_pool_cancel(BlockDriverAIOCB *= acb) > */ > qemu_sem_timedwait(&pool->sem, 0) =3D=3D 0) { > QTAILQ_REMOVE(&pool->request_list, elem, reqs); > - elem->state =3D THREAD_CANCELED; > qemu_bh_schedule(pool->completion_bh); > - } else { > - pool->pending_cancellations++; > - while (elem->state !=3D THREAD_CANCELED && elem->state !=3D TH= READ_DONE) { > - qemu_cond_wait(&pool->check_cancel, &pool->lock); > - } > - pool->pending_cancellations--; > + > + elem->state =3D THREAD_DONE; > + elem->ret =3D -ECANCELED; > } > + > qemu_mutex_unlock(&pool->lock); > - thread_pool_completion_bh(pool); > +} > + > +static AioContext *thread_pool_get_aio_context(BlockDriverAIOCB *acb) > +{ > + ThreadPoolElement *elem =3D (ThreadPoolElement *)acb; > + ThreadPool *pool =3D elem->pool; > + return pool->ctx; > } > =20 > static const AIOCBInfo thread_pool_aiocb_info =3D { > .aiocb_size =3D sizeof(ThreadPoolElement), > - .cancel =3D thread_pool_cancel, > + .cancel_async =3D thread_pool_cancel, > + .get_aio_context =3D thread_pool_get_aio_context, > }; > =20 > BlockDriverAIOCB *thread_pool_submit_aio(ThreadPool *pool, > @@ -300,7 +298,6 @@ static void thread_pool_init_one(ThreadPool *pool, = AioContext *ctx) > pool->ctx =3D ctx; > pool->completion_bh =3D aio_bh_new(ctx, thread_pool_completion_bh,= pool); > qemu_mutex_init(&pool->lock); > - qemu_cond_init(&pool->check_cancel); > qemu_cond_init(&pool->worker_stopped); > qemu_sem_init(&pool->sem, 0); > pool->max_threads =3D 64; > @@ -343,7 +340,6 @@ void thread_pool_free(ThreadPool *pool) > =20 > qemu_bh_delete(pool->completion_bh); > qemu_sem_destroy(&pool->sem); > - qemu_cond_destroy(&pool->check_cancel); > qemu_cond_destroy(&pool->worker_stopped); > qemu_mutex_destroy(&pool->lock); > g_free(pool); > --=20 > 2.1.0.27.g96db324 >=20 >=20