From patchwork Sat Apr 17 21:32:21 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 50387 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 0E74BB7D0B for ; Sun, 18 Apr 2010 07:33:40 +1000 (EST) Received: from localhost ([127.0.0.1]:45013 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O3Fdw-0004Jl-Ad for incoming@patchwork.ozlabs.org; Sat, 17 Apr 2010 17:33:36 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O3Fco-000485-Ge for qemu-devel@nongnu.org; Sat, 17 Apr 2010 17:32:26 -0400 Received: from [140.186.70.92] (port=53398 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O3Fcm-00047P-ES for qemu-devel@nongnu.org; Sat, 17 Apr 2010 17:32:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O3Fck-0000Jw-I6 for qemu-devel@nongnu.org; Sat, 17 Apr 2010 17:32:24 -0400 Received: from mail-qy0-f181.google.com ([209.85.221.181]:53459) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O3Fck-0000Jp-Bu for qemu-devel@nongnu.org; Sat, 17 Apr 2010 17:32:22 -0400 Received: by qyk11 with SMTP id 11so3625127qyk.13 for ; Sat, 17 Apr 2010 14:32:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:received:message-id:subject:from:to:cc:content-type; bh=KK7oPt48eKMVQMCisFYEVabXWTp6553gf1C6G/2vus8=; b=oMij3nkJPYdvIwsVyPDLubCWcPXumVGigYVtqhnrIxKkD2svcROlCpDYHLRcu9nNE6 Z11pzB2Q2sSjNqhq3Z7tRgzZNRQLupDKn9lFZCIxppw8BvvId/qIZEiO8hnFiqSrCdWM jhWI2sws/7WrztP310LzMAenZzPiR2IlmrnmU= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=SYLVFhYWj6qEYSaSNJxrOnBFwja5qnOQQkKMW6kfxE+QvDFhjRrzoHqRn3WrOg6QRi uerkVIRTNzt/4IAciSL19oVx7hVM+jsF8uKfmNZg4id5TUUdcjjkzvhCUczmm99JzRe1 emQXs2oIjDp/Ed2zEtY+tw5UTSfYAQtSA+RUw= MIME-Version: 1.0 Received: by 10.229.212.3 with HTTP; Sat, 17 Apr 2010 14:32:21 -0700 (PDT) In-Reply-To: <20100417194009.GA13862@lst.de> References: <4BC757DD.8060804@siemens.com> <20100417194009.GA13862@lst.de> Date: Sat, 17 Apr 2010 22:32:21 +0100 Received: by 10.229.248.137 with SMTP id mg9mr4778247qcb.33.1271539941744; Sat, 17 Apr 2010 14:32:21 -0700 (PDT) Message-ID: Subject: Re: [Qemu-devel] Guest latency issues due to bdrv_check_byte_request From: Stefan Hajnoczi To: Christoph Hellwig X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Cc: Jan Kiszka , qemu-devel X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Thanks Christoph. Cached getlength with pread/pwrite: % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 96.97 1.760111 11893 148 4 futex 1.61 0.029209 1 46891 2217 select 0.28 0.005047 0 64609 timer_gettime 0.22 0.004059 0 42745 2578 rt_sigreturn 0.22 0.003911 0 46261 timer_settime 0.18 0.003280 1093 3 shmdt 0.17 0.003095 0 23859 pread <--- 0.17 0.003061 0 42800 write 0.16 0.002916 0 47759 5151 read 0.02 0.000285 0 645 writev [...] 0.00 0.000000 0 13 lseek Note that this is a Tiny Core Linux boot from disk and shutdown; not very I/O intensive since it only loads a kernel and ~10 MB initramfs without touching the disk much after kernel load. if (bdrv_flags & BDRV_O_RDWR) { @@ -243,19 +240,7 @@ static int raw_pread_aligned(BlockDriverState *bs, int64_t offset, if (ret < 0) return ret; - if (offset >= 0 && lseek(s->fd, offset, SEEK_SET) == (off_t)-1) { - ++(s->lseek_err_cnt); - if(s->lseek_err_cnt <= 10) { - DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64 - "] lseek failed : %d = %s\n", - s->fd, bs->filename, offset, buf, count, - bs->total_sectors, errno, strerror(errno)); - } - return -1; - } - s->lseek_err_cnt=0; - - ret = read(s->fd, buf, count); + ret = pread(s->fd, buf, count, offset); if (ret == count) goto label__raw_read__success; @@ -276,12 +261,10 @@ static int raw_pread_aligned(BlockDriverState *bs, int64_t offset, /* Try harder for CDrom. */ if (bs->type == BDRV_TYPE_CDROM) { - lseek(s->fd, offset, SEEK_SET); - ret = read(s->fd, buf, count); + ret = pread(s->fd, buf, count, offset); if (ret == count) goto label__raw_read__success; - lseek(s->fd, offset, SEEK_SET); - ret = read(s->fd, buf, count); + ret = pread(s->fd, buf, count, offset); if (ret == count) goto label__raw_read__success; @@ -313,19 +296,7 @@ static int raw_pwrite_aligned(BlockDriverState *bs, int64_t offset, if (ret < 0) return -errno; - if (offset >= 0 && lseek(s->fd, offset, SEEK_SET) == (off_t)-1) { - ++(s->lseek_err_cnt); - if(s->lseek_err_cnt) { - DEBUG_BLOCK_PRINT("raw_pwrite(%d:%s, %" PRId64 ", %p, %d) [%" - PRId64 "] lseek failed : %d = %s\n", - s->fd, bs->filename, offset, buf, count, - bs->total_sectors, errno, strerror(errno)); - } - return -EIO; - } - s->lseek_err_cnt = 0; - - ret = write(s->fd, buf, count); + ret = pwrite(s->fd, buf, count, offset); if (ret == count) goto label__raw_write__success; Stefan diff --git a/block.c b/block.c index 0f6be17..5c1652c 100644 --- a/block.c +++ b/block.c @@ -363,6 +363,7 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename, assert(drv != NULL); bs->file = NULL; + bs->total_sectors = 0; bs->is_temporary = 0; bs->encrypted = 0; bs->valid_key = 0; @@ -416,9 +417,7 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename, } bs->keep_read_only = bs->read_only = !(open_flags & BDRV_O_RDWR); - if (drv->bdrv_getlength) { - bs->total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS; - } + bs->total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS; #ifndef _WIN32 if (bs->is_temporary) { unlink(filename); @@ -957,13 +956,26 @@ int bdrv_pwrite(BlockDriverState *bs, int64_t offset, int bdrv_truncate(BlockDriverState *bs, int64_t offset) { BlockDriver *drv = bs->drv; + int ret; if (!drv) return -ENOMEDIUM; if (!drv->bdrv_truncate) return -ENOTSUP; if (bs->read_only) return -EACCES; - return drv->bdrv_truncate(bs, offset); + ret = drv->bdrv_truncate(bs, offset); + if (ret < 0) { + return ret; + } + + /* refresh total sectors */ + if (drv->bdrv_getlength) { + bs->total_sectors = 0; /* discard cached value */ + bs->total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS; + } else { + bs->total_sectors = offset >> BDRV_SECTOR_BITS; + } + return ret; } /** @@ -974,8 +986,12 @@ int64_t bdrv_getlength(BlockDriverState *bs) BlockDriver *drv = bs->drv; if (!drv) return -ENOMEDIUM; - if (!drv->bdrv_getlength) { - /* legacy mode */ + + /* Fixed size devices use the total_sectors value for speed instead of + issuing a length query (like lseek) on each call. Also, legacy block + drivers don't provide a bdrv_getlength function and must use + total_sectors. */ + if ((bs->total_sectors && !bs->growable) || !drv->bdrv_getlength) { return bs->total_sectors * BDRV_SECTOR_SIZE; } return drv->bdrv_getlength(bs); diff --git a/block/raw-posix.c b/block/raw-posix.c index 598ea19..7541ed2 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -105,7 +105,6 @@ typedef struct BDRVRawState { int fd; int type; - unsigned int lseek_err_cnt; int open_flags; #if defined(__linux__) /* linux floppy specific */ @@ -134,8 +133,6 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, BDRVRawState *s = bs->opaque; int fd, ret; - s->lseek_err_cnt = 0; - s->open_flags = open_flags | O_BINARY; s->open_flags &= ~O_ACCMODE;