From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55628) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RDW5F-0002QI-KE for qemu-devel@nongnu.org; Tue, 11 Oct 2011 02:45:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RDW5E-0003N1-Fg for qemu-devel@nongnu.org; Tue, 11 Oct 2011 02:45:01 -0400 Received: from mail-wy0-f173.google.com ([74.125.82.173]:35002) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RDW5E-0003Mo-2a for qemu-devel@nongnu.org; Tue, 11 Oct 2011 02:45:00 -0400 Received: by wyh22 with SMTP id 22so7637063wyh.4 for ; Mon, 10 Oct 2011 23:44:59 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1317831427-477-4-git-send-email-stefanha@linux.vnet.ibm.com> References: <1317831427-477-1-git-send-email-stefanha@linux.vnet.ibm.com> <1317831427-477-4-git-send-email-stefanha@linux.vnet.ibm.com> Date: Tue, 11 Oct 2011 14:44:58 +0800 Message-ID: From: Zhi Yong Wu Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/6] block: switch bdrv_read()/bdrv_write() to coroutines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Anthony Liguori , Marcelo Tosatti , qemu-devel@nongnu.org, Zhi Yong Wu , Avi Kivity , Christoph Hellwig On Thu, Oct 6, 2011 at 12:17 AM, Stefan Hajnoczi wrote: > The bdrv_read()/bdrv_write() functions call .bdrv_read()/.bdrv_write(). > They should go through bdrv_co_do_readv() and bdrv_co_do_writev() > instead in order to unify request processing code across sync, aio, and > coroutine interfaces. =A0This is also an important step towards removing > BlockDriverState .bdrv_read()/.bdrv_write() in the future. > > Signed-off-by: Stefan Hajnoczi > --- > =A0block.c | =A0112 +++++++++++++++++++++++++++++++++++------------------= ---------- > =A01 files changed, 62 insertions(+), 50 deletions(-) > > diff --git a/block.c b/block.c > index d15784e..90c29db 100644 > --- a/block.c > +++ b/block.c > @@ -44,6 +44,8 @@ > =A0#include > =A0#endif > > +#define NOT_DONE 0x7fffffff /* used while emulated sync operation in pro= gress */ > + > =A0static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load); > =A0static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs, > =A0 =A0 =A0 =A0 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, > @@ -74,6 +76,8 @@ static int coroutine_fn bdrv_co_writev_em(BlockDriverSt= ate *bs, > =A0static int coroutine_fn bdrv_co_flush_em(BlockDriverState *bs); > =A0static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs, > =A0 =A0 int64_t sector_num, int nb_sectors, QEMUIOVector *qiov); > +static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs, > + =A0 =A0int64_t sector_num, int nb_sectors, QEMUIOVector *qiov); > > =A0static QTAILQ_HEAD(, BlockDriverState) bdrv_states =3D > =A0 =A0 QTAILQ_HEAD_INITIALIZER(bdrv_states); > @@ -1038,30 +1042,69 @@ static inline bool bdrv_has_async_flush(BlockDriv= er *drv) > =A0 =A0 return drv->bdrv_aio_flush !=3D bdrv_aio_flush_em; > =A0} > > -/* return < 0 if error. See bdrv_write() for the return codes */ > -int bdrv_read(BlockDriverState *bs, int64_t sector_num, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0uint8_t *buf, int nb_sectors) > +typedef struct RwCo { > + =A0 =A0BlockDriverState *bs; > + =A0 =A0int64_t sector_num; > + =A0 =A0int nb_sectors; > + =A0 =A0QEMUIOVector *qiov; > + =A0 =A0bool is_write; > + =A0 =A0int ret; > +} RwCo; > + > +static void coroutine_fn bdrv_rw_co_entry(void *opaque) > =A0{ > - =A0 =A0BlockDriver *drv =3D bs->drv; > + =A0 =A0RwCo *rwco =3D opaque; > > - =A0 =A0if (!drv) > - =A0 =A0 =A0 =A0return -ENOMEDIUM; > + =A0 =A0if (!rwco->is_write) { > + =A0 =A0 =A0 =A0rwco->ret =3D bdrv_co_do_readv(rwco->bs, rwco->sector_nu= m, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= rwco->nb_sectors, rwco->qiov); > + =A0 =A0} else { > + =A0 =A0 =A0 =A0rwco->ret =3D bdrv_co_do_writev(rwco->bs, rwco->sector_n= um, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0rwco->nb_sectors, rwco->qiov); > + =A0 =A0} > +} > > - =A0 =A0if (bdrv_has_async_rw(drv) && qemu_in_coroutine()) { > - =A0 =A0 =A0 =A0QEMUIOVector qiov; > - =A0 =A0 =A0 =A0struct iovec iov =3D { > - =A0 =A0 =A0 =A0 =A0 =A0.iov_base =3D (void *)buf, > - =A0 =A0 =A0 =A0 =A0 =A0.iov_len =3D nb_sectors * BDRV_SECTOR_SIZE, > - =A0 =A0 =A0 =A0}; > +/* > + * Process a synchronous request using coroutines > + */ > +static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t = *buf, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int nb_sectors, bool is_writ= e) > +{ > + =A0 =A0QEMUIOVector qiov; > + =A0 =A0struct iovec iov =3D { > + =A0 =A0 =A0 =A0.iov_base =3D (void *)buf, > + =A0 =A0 =A0 =A0.iov_len =3D nb_sectors * BDRV_SECTOR_SIZE, > + =A0 =A0}; > + =A0 =A0Coroutine *co; > + =A0 =A0RwCo rwco =3D { > + =A0 =A0 =A0 =A0.bs =3D bs, > + =A0 =A0 =A0 =A0.sector_num =3D sector_num, > + =A0 =A0 =A0 =A0.nb_sectors =3D nb_sectors, > + =A0 =A0 =A0 =A0.qiov =3D &qiov, > + =A0 =A0 =A0 =A0.is_write =3D is_write, > + =A0 =A0 =A0 =A0.ret =3D NOT_DONE, > + =A0 =A0}; > > - =A0 =A0 =A0 =A0qemu_iovec_init_external(&qiov, &iov, 1); > - =A0 =A0 =A0 =A0return bdrv_co_readv(bs, sector_num, nb_sectors, &qiov); > - =A0 =A0} > + =A0 =A0qemu_iovec_init_external(&qiov, &iov, 1); > > - =A0 =A0if (bdrv_check_request(bs, sector_num, nb_sectors)) > - =A0 =A0 =A0 =A0return -EIO; > + =A0 =A0if (qemu_in_coroutine()) { > + =A0 =A0 =A0 =A0/* Fast-path if already in coroutine context */ > + =A0 =A0 =A0 =A0bdrv_rw_co_entry(&rwco); > + =A0 =A0} else { > + =A0 =A0 =A0 =A0co =3D qemu_coroutine_create(bdrv_rw_co_entry); > + =A0 =A0 =A0 =A0qemu_coroutine_enter(co, &rwco); > + =A0 =A0 =A0 =A0while (rwco.ret =3D=3D NOT_DONE) { > + =A0 =A0 =A0 =A0 =A0 =A0qemu_aio_wait(); > + =A0 =A0 =A0 =A0} > + =A0 =A0} > + =A0 =A0return rwco.ret; > +} > > - =A0 =A0return drv->bdrv_read(bs, sector_num, buf, nb_sectors); > +/* return < 0 if error. See bdrv_write() for the return codes */ > +int bdrv_read(BlockDriverState *bs, int64_t sector_num, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0uint8_t *buf, int nb_sectors) > +{ > + =A0 =A0return bdrv_rw_co(bs, sector_num, buf, nb_sectors, false); > =A0} > > =A0static void set_dirty_bitmap(BlockDriverState *bs, int64_t sector_num, > @@ -1101,36 +1144,7 @@ static void set_dirty_bitmap(BlockDriverState *bs,= int64_t sector_num, > =A0int bdrv_write(BlockDriverState *bs, int64_t sector_num, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const uint8_t *buf, int nb_sectors) > =A0{ > - =A0 =A0BlockDriver *drv =3D bs->drv; > - > - =A0 =A0if (!bs->drv) > - =A0 =A0 =A0 =A0return -ENOMEDIUM; > - > - =A0 =A0if (bdrv_has_async_rw(drv) && qemu_in_coroutine()) { > - =A0 =A0 =A0 =A0QEMUIOVector qiov; > - =A0 =A0 =A0 =A0struct iovec iov =3D { > - =A0 =A0 =A0 =A0 =A0 =A0.iov_base =3D (void *)buf, > - =A0 =A0 =A0 =A0 =A0 =A0.iov_len =3D nb_sectors * BDRV_SECTOR_SIZE, > - =A0 =A0 =A0 =A0}; > - > - =A0 =A0 =A0 =A0qemu_iovec_init_external(&qiov, &iov, 1); > - =A0 =A0 =A0 =A0return bdrv_co_writev(bs, sector_num, nb_sectors, &qiov)= ; > - =A0 =A0} > - > - =A0 =A0if (bs->read_only) > - =A0 =A0 =A0 =A0return -EACCES; > - =A0 =A0if (bdrv_check_request(bs, sector_num, nb_sectors)) > - =A0 =A0 =A0 =A0return -EIO; > - > - =A0 =A0if (bs->dirty_bitmap) { > - =A0 =A0 =A0 =A0set_dirty_bitmap(bs, sector_num, nb_sectors, 1); > - =A0 =A0} > - > - =A0 =A0if (bs->wr_highest_sector < sector_num + nb_sectors - 1) { > - =A0 =A0 =A0 =A0bs->wr_highest_sector =3D sector_num + nb_sectors - 1; > - =A0 =A0} The above codes are removed, will it be safe? > - > - =A0 =A0return drv->bdrv_write(bs, sector_num, buf, nb_sectors); > + =A0 =A0return bdrv_rw_co(bs, sector_num, (uint8_t *)buf, nb_sectors, tr= ue); > =A0} > > =A0int bdrv_pread(BlockDriverState *bs, int64_t offset, > @@ -2891,8 +2905,6 @@ static void bdrv_rw_em_cb(void *opaque, int ret) > =A0 =A0 *(int *)opaque =3D ret; > =A0} > > -#define NOT_DONE 0x7fffffff > - > =A0static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 uint8_t *buf, int nb_sect= ors) > =A0{ > -- > 1.7.6.3 > > > --=20 Regards, Zhi Yong Wu