From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ryan Harper Subject: Re: [Qemu-devel] [PATCH v4 2/3] The support for block queue Date: Mon, 1 Aug 2011 15:21:26 -0500 Message-ID: <20110801202126.GM7358@us.ibm.com> References: <1312179955-23536-1-git-send-email-wuzhy@linux.vnet.ibm.com> <1312179955-23536-3-git-send-email-wuzhy@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: qemu-devel@nongnu.org, kwolf@redhat.com, aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, kvm@vger.kernel.org, mtosatti@redhat.com, zwu.kernel@gmail.com, luowenj@cn.ibm.com, raharper@us.ibm.com To: Zhi Yong Wu Return-path: Received: from e3.ny.us.ibm.com ([32.97.182.143]:38567 "EHLO e3.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753260Ab1HAUVc (ORCPT ); Mon, 1 Aug 2011 16:21:32 -0400 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by e3.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p71JvhNS022295 for ; Mon, 1 Aug 2011 15:57:43 -0400 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p71KLVgM1409212 for ; Mon, 1 Aug 2011 16:21:31 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p71GLH6o000646 for ; Mon, 1 Aug 2011 13:21:18 -0300 Content-Disposition: inline In-Reply-To: <1312179955-23536-3-git-send-email-wuzhy@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: * Zhi Yong Wu [2011-08-01 01:30]: > +static AIOPool block_queue_pool = { > + .aiocb_size = sizeof(struct BlockDriverAIOCB), > + .cancel = qemu_block_queue_cancel, > +}; > + > +static void qemu_block_queue_callback(void *opaque, int ret) > +{ > + BlockDriverAIOCB *acb = opaque; > + > + qemu_aio_release(acb); > +} > + So, here we really want to invoke the original commands callback, and then free the request here (via qemu_aio_release()). see below. > +BlockDriverAIOCB *qemu_block_queue_enqueue(BlockQueue *queue, > + BlockDriverState *bs, > + BlockRequestHandler *handler, > + int64_t sector_num, > + QEMUIOVector *qiov, > + int nb_sectors, > + BlockDriverCompletionFunc *cb, > + void *opaque) > +{ > + BlockIORequest *request; > + BlockDriverAIOCB *acb; > + > + request = qemu_malloc(sizeof(BlockIORequest)); > + request->bs = bs; > + request->handler = handler; > + request->sector_num = sector_num; > + request->qiov = qiov; > + request->nb_sectors = nb_sectors; > + request->cb = cb; > + request->opaque = opaque; > + > + QTAILQ_INSERT_TAIL(&queue->requests, request, entry); > + > + acb = qemu_aio_get(&block_queue_pool, bs, > + qemu_block_queue_callback, opaque); > + request->acb = acb; > + > + return acb; > +} > + > +int qemu_block_queue_handler(BlockIORequest *request) > +{ > + int ret; > + BlockDriverAIOCB *res; > + > + /* indicate this req is from block queue */ > + request->bs->req_from_queue = true; > + > + res = request->handler(request->bs, request->sector_num, > + request->qiov, request->nb_sectors, > + request->cb, request->opaque); > + > + if (request->acb) { > + qemu_block_queue_callback(request->acb, 0); > + } > + > + ret = (res == NULL) ? 0 : 1; > + > + return ret; > +} You don't want to malloc the BlockIORequest directly in _enqueue(), rather you want to allocate that from your AIOPool via qemu_aio_get(). As it is now, we're allocating two BlockIORequests (malloc and then a aio_get()). You'll need a BlockDriverAIOCB in the BlockIORequest structure. Then in your queue_handler, instead of passing the original read or write callback (request->cb), you want to hook the block_queue callback (qemu_block_queue_callback()), in that callback you can then invoke the request callback and then release the request. The request should be, only one malloc (via the pool which will re-use the memory instead of incuring a malloc on every request), and then you release the memory back to the pool once your request is complete, which you'll know after wiring up the block_queue callback to the completion of the request's handler. And then since we don't double allocate, you won't need to do the qemu_free(request) in block.c in block_timer... -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh@us.ibm.com From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45480) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QnyzW-0005c6-87 for qemu-devel@nongnu.org; Mon, 01 Aug 2011 16:21:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QnyzV-0002Bi-6q for qemu-devel@nongnu.org; Mon, 01 Aug 2011 16:21:34 -0400 Received: from e5.ny.us.ibm.com ([32.97.182.145]:42669) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QnyzV-0002Be-35 for qemu-devel@nongnu.org; Mon, 01 Aug 2011 16:21:33 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e5.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p71JqDMw018595 for ; Mon, 1 Aug 2011 15:52:13 -0400 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p71KLVRY140588 for ; Mon, 1 Aug 2011 16:21:31 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p71GLH6i000646 for ; Mon, 1 Aug 2011 13:21:18 -0300 Date: Mon, 1 Aug 2011 15:21:26 -0500 From: Ryan Harper Message-ID: <20110801202126.GM7358@us.ibm.com> References: <1312179955-23536-1-git-send-email-wuzhy@linux.vnet.ibm.com> <1312179955-23536-3-git-send-email-wuzhy@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1312179955-23536-3-git-send-email-wuzhy@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v4 2/3] The support for block queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhi Yong Wu Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, kvm@vger.kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, zwu.kernel@gmail.com, luowenj@cn.ibm.com, raharper@us.ibm.com * Zhi Yong Wu [2011-08-01 01:30]: > +static AIOPool block_queue_pool = { > + .aiocb_size = sizeof(struct BlockDriverAIOCB), > + .cancel = qemu_block_queue_cancel, > +}; > + > +static void qemu_block_queue_callback(void *opaque, int ret) > +{ > + BlockDriverAIOCB *acb = opaque; > + > + qemu_aio_release(acb); > +} > + So, here we really want to invoke the original commands callback, and then free the request here (via qemu_aio_release()). see below. > +BlockDriverAIOCB *qemu_block_queue_enqueue(BlockQueue *queue, > + BlockDriverState *bs, > + BlockRequestHandler *handler, > + int64_t sector_num, > + QEMUIOVector *qiov, > + int nb_sectors, > + BlockDriverCompletionFunc *cb, > + void *opaque) > +{ > + BlockIORequest *request; > + BlockDriverAIOCB *acb; > + > + request = qemu_malloc(sizeof(BlockIORequest)); > + request->bs = bs; > + request->handler = handler; > + request->sector_num = sector_num; > + request->qiov = qiov; > + request->nb_sectors = nb_sectors; > + request->cb = cb; > + request->opaque = opaque; > + > + QTAILQ_INSERT_TAIL(&queue->requests, request, entry); > + > + acb = qemu_aio_get(&block_queue_pool, bs, > + qemu_block_queue_callback, opaque); > + request->acb = acb; > + > + return acb; > +} > + > +int qemu_block_queue_handler(BlockIORequest *request) > +{ > + int ret; > + BlockDriverAIOCB *res; > + > + /* indicate this req is from block queue */ > + request->bs->req_from_queue = true; > + > + res = request->handler(request->bs, request->sector_num, > + request->qiov, request->nb_sectors, > + request->cb, request->opaque); > + > + if (request->acb) { > + qemu_block_queue_callback(request->acb, 0); > + } > + > + ret = (res == NULL) ? 0 : 1; > + > + return ret; > +} You don't want to malloc the BlockIORequest directly in _enqueue(), rather you want to allocate that from your AIOPool via qemu_aio_get(). As it is now, we're allocating two BlockIORequests (malloc and then a aio_get()). You'll need a BlockDriverAIOCB in the BlockIORequest structure. Then in your queue_handler, instead of passing the original read or write callback (request->cb), you want to hook the block_queue callback (qemu_block_queue_callback()), in that callback you can then invoke the request callback and then release the request. The request should be, only one malloc (via the pool which will re-use the memory instead of incuring a malloc on every request), and then you release the memory back to the pool once your request is complete, which you'll know after wiring up the block_queue callback to the completion of the request's handler. And then since we don't double allocate, you won't need to do the qemu_free(request) in block.c in block_timer... -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh@us.ibm.com