From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59488) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qwczd-0003Ce-As for qemu-devel@nongnu.org; Thu, 25 Aug 2011 12:41:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qwczb-0004ha-VU for qemu-devel@nongnu.org; Thu, 25 Aug 2011 12:41:25 -0400 Received: from e5.ny.us.ibm.com ([32.97.182.145]:42264) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qwczb-0004hT-Sx for qemu-devel@nongnu.org; Thu, 25 Aug 2011 12:41:23 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by e5.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p7PGBLTq016466 for ; Thu, 25 Aug 2011 12:11:21 -0400 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p7PGfMjf208152 for ; Thu, 25 Aug 2011 12:41:22 -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 p7PCf9mS028322 for ; Thu, 25 Aug 2011 09:41:09 -0300 Date: Thu, 25 Aug 2011 11:41:18 -0500 From: Ryan Harper Message-ID: <20110825164118.GA30100@us.ibm.com> References: <20110821222547.GA22046@lst.de> <20110822145916.GS5792@us.ibm.com> <20110822151208.GB4130@lst.de> <20110822152911.GT5792@us.ibm.com> <20110822153514.GA4749@lst.de> <20110822164600.GU5792@us.ibm.com> <20110824184506.GA20556@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110824184506.GA20556@lst.de> Subject: Re: [Qemu-devel] [PATCH 0/3] better I/O accounting V2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christoph Hellwig Cc: Ryan Harper , qemu-devel@nongnu.org * Christoph Hellwig [2011-08-24 13:46]: > On Mon, Aug 22, 2011 at 11:46:00AM -0500, Ryan Harper wrote: > > So, I believe this is how it's happening. > > > > we start accounting on a write which is turned into a multiwrite via > > virtio_blk_handle_write() which calls virtio_submit_multiwrite(). > > > > Then when the multiwrite completes, we invoke virtio_blk_rw_complete() > > on each part of the multiwrite. None of these requests have had their > > acct structure initialized since there was just *one* initial write. > > We could do a bdrv_acct_start() on each req, but that would break the > > concept of hiding the additional writes under the initial request. > > We do initialize all write fields correctly. Where we fail right now > is for non-read/write requests. I can reproduce your issue by reading > the serial attribute in sysfs - any chance Fedora does that during boot? Indeed. F15 has udev hooks to support /dev/disk/by-id for virtio-blk devices which will issue non-read/write request (get drive serial). > > The patch below should take care of these cases: Tested with this patch, F15 boots fine, mkfs.ext4 and dd all work as expected. Tested-by: Ryan Harper > > > Index: qemu/hw/virtio-blk.c > =================================================================== > --- qemu.orig/hw/virtio-blk.c 2011-08-24 20:32:03.764503179 +0200 > +++ qemu/hw/virtio-blk.c 2011-08-24 20:35:55.716579922 +0200 > @@ -59,9 +59,6 @@ static void virtio_blk_req_complete(Virt > stb_p(&req->in->status, status); > virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in)); > virtio_notify(&s->vdev, s->vq); > - > - bdrv_acct_done(s->bs, &req->acct); > - g_free(req); > } > > static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, > @@ -83,6 +80,8 @@ static int virtio_blk_handle_rw_error(Vi > vm_stop(VMSTOP_DISKFULL); > } else { > virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > + bdrv_acct_done(s->bs, &req->acct); > + g_free(req); > bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read); > } > > @@ -102,6 +101,8 @@ static void virtio_blk_rw_complete(void > } > > virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); > + bdrv_acct_done(req->dev->bs, &req->acct); > + g_free(req); > } > > static void virtio_blk_flush_complete(void *opaque, int ret) > @@ -115,6 +116,8 @@ static void virtio_blk_flush_complete(vo > } > > virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); > + bdrv_acct_done(req->dev->bs, &req->acct); > + g_free(req); > } > > static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) > @@ -157,6 +160,7 @@ static void virtio_blk_handle_scsi(VirtI > */ > if (req->elem.out_num < 2 || req->elem.in_num < 3) { > virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > + g_free(req); > return; > } > > @@ -165,6 +169,7 @@ static void virtio_blk_handle_scsi(VirtI > */ > if (req->elem.out_num > 2 && req->elem.in_num > 3) { > virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); > + g_free(req); > return; > } > > @@ -231,11 +236,13 @@ static void virtio_blk_handle_scsi(VirtI > stl_p(&req->scsi->data_len, hdr.dxfer_len); > > virtio_blk_req_complete(req, status); > + g_free(req); > } > #else > static void virtio_blk_handle_scsi(VirtIOBlockReq *req) > { > virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); > + g_free(req); > } > #endif /* __linux__ */ > > @@ -378,6 +385,7 @@ static void virtio_blk_handle_request(Vi > s->serial ? s->serial : "", > MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES)); > virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); > + g_free(req); > } else if (type & VIRTIO_BLK_T_OUT) { > qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1], > req->elem.out_num - 1); -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh@us.ibm.com