From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33005) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHomG-0001Tc-Kn for qemu-devel@nongnu.org; Tue, 28 Jun 2016 04:53:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHomC-0001bC-F4 for qemu-devel@nongnu.org; Tue, 28 Jun 2016 04:53:51 -0400 Received: from 11.mo3.mail-out.ovh.net ([87.98.184.158]:52522) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHomC-0001b7-3V for qemu-devel@nongnu.org; Tue, 28 Jun 2016 04:53:48 -0400 Received: from player772.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo3.mail-out.ovh.net (Postfix) with ESMTP id 5400EFF9398 for ; Tue, 28 Jun 2016 10:53:45 +0200 (CEST) References: <1467103170-5784-1-git-send-email-pbonzini@redhat.com> <1467103170-5784-2-git-send-email-pbonzini@redhat.com> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: Date: Tue, 28 Jun 2016 10:53:39 +0200 MIME-Version: 1.0 In-Reply-To: <1467103170-5784-2-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] m25p80: do not put iovec on the stack List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org On 06/28/2016 10:39 AM, Paolo Bonzini wrote: > When doing a read-modify-write cycle, QEMU uses the iovec after returning > from blk_aio_pwritev. m25p80 puts the iovec on the stack of blk_aio_pwritev's > caller, which causes trouble in this case. This has been a problem > since commit 243e6f6 ("m25p80: Switch to byte-based block access", > 2016-05-12) started doing writes at a smaller granularity than 512 bytes. > In principle however it could have broken before when using -drive > if=mtd,cache=none on a disk with 4K native sectors. Ah ! Thanks. That was a problem I was seeing. I was thinking we could just do synchronous writes : https://github.com/legoater/qemu/commit/aef3fe4db3be632077c581541fe30b4e36b5a6f7 and enable snapshotting like : https://github.com/legoater/qemu/commit/b90ccab7873fd3538b47396ec7c3ae35c8e13270 Thanks, C. > Signed-off-by: Paolo Bonzini > --- > hw/block/m25p80.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c > index 09b4767..dd6714d 100644 > --- a/hw/block/m25p80.c > +++ b/hw/block/m25p80.c > @@ -446,6 +446,11 @@ static inline Manufacturer get_man(Flash *s) > > static void blk_sync_complete(void *opaque, int ret) > { > + QEMUIOVector *iov = opaque; > + > + qemu_iovec_destroy(iov); > + g_free(iov); > + > /* do nothing. Masters do not directly interact with the backing store, > * only the working copy so no mutexing required. > */ > @@ -453,31 +458,31 @@ static void blk_sync_complete(void *opaque, int ret) > > static void flash_sync_page(Flash *s, int page) > { > - QEMUIOVector iov; > + QEMUIOVector *iov = g_new(QEMUIOVector, 1); > > if (!s->blk || blk_is_read_only(s->blk)) { > return; > } > > - qemu_iovec_init(&iov, 1); > - qemu_iovec_add(&iov, s->storage + page * s->pi->page_size, > + qemu_iovec_init(iov, 1); > + qemu_iovec_add(iov, s->storage + page * s->pi->page_size, > s->pi->page_size); > - blk_aio_pwritev(s->blk, page * s->pi->page_size, &iov, 0, > - blk_sync_complete, NULL); > + blk_aio_pwritev(s->blk, page * s->pi->page_size, iov, 0, > + blk_sync_complete, iov); > } > > static inline void flash_sync_area(Flash *s, int64_t off, int64_t len) > { > - QEMUIOVector iov; > + QEMUIOVector *iov = g_new(QEMUIOVector, 1); > > if (!s->blk || blk_is_read_only(s->blk)) { > return; > } > > assert(!(len % BDRV_SECTOR_SIZE)); > - qemu_iovec_init(&iov, 1); > - qemu_iovec_add(&iov, s->storage + off, len); > - blk_aio_pwritev(s->blk, off, &iov, 0, blk_sync_complete, NULL); > + qemu_iovec_init(iov, 1); > + qemu_iovec_add(iov, s->storage + off, len); > + blk_aio_pwritev(s->blk, off, iov, 0, blk_sync_complete, iov); > } > > static void flash_erase(Flash *s, int offset, FlashCMD cmd) >