From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O3sJo-0003aD-NV for qemu-devel@nongnu.org; Mon, 19 Apr 2010 10:51:24 -0400 Received: from [140.186.70.92] (port=36388 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O3sJn-0003Zc-9O for qemu-devel@nongnu.org; Mon, 19 Apr 2010 10:51:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O3sJi-0006Ws-6c for qemu-devel@nongnu.org; Mon, 19 Apr 2010 10:51:23 -0400 Received: from mail-gy0-f173.google.com ([209.85.160.173]:35701) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O3sJh-0006WP-LA for qemu-devel@nongnu.org; Mon, 19 Apr 2010 10:51:18 -0400 Received: by gyd5 with SMTP id 5so2583252gyd.4 for ; Mon, 19 Apr 2010 07:51:16 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4BCC44FD.8000008@redhat.com> References: <1271670198-12793-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1271670198-12793-3-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <4BCC2DBA.4000602@redhat.com> <4BCC3226.20305@lab.ntt.co.jp> <4BCC40C6.1040203@redhat.com> <4BCC43FA.3090704@lab.ntt.co.jp> <4BCC44FD.8000008@redhat.com> Date: Mon, 19 Apr 2010 23:51:04 +0900 Message-ID: Subject: Re: [Qemu-devel] Re: [PATCH v3 2/6] Introduce bit-based phys_ram_dirty for VGA, CODE, MIGRATION and MASTER. From: Yoshiaki Tamura Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: aliguori@us.ibm.com, mtosatti@redhat.com, qemu-devel@nongnu.org, ohmura.kei@lab.ntt.co.jp 2010/4/19 Avi Kivity : > On 04/19/2010 02:52 PM, Yoshiaki Tamura wrote: >> >> Avi Kivity wrote: >>> >>> On 04/19/2010 02:31 PM, Yoshiaki Tamura wrote: >>>> >>>>>>> typedef struct RAMBlock { >>>>>>> @@ -2825,10 +2825,16 @@ ram_addr_t qemu_ram_alloc(ram_addr_t size) >>>>>>> new_block->next =3D ram_blocks; >>>>>>> ram_blocks =3D new_block; >>>>>>> >>>>>>> - phys_ram_dirty =3D qemu_realloc(phys_ram_dirty, >>>>>>> - (last_ram_offset + size)>> TARGET_PAGE_BITS); >>>>>>> - memset(phys_ram_dirty + (last_ram_offset>> TARGET_PAGE_BITS), >>>>>>> - 0xff, size>> TARGET_PAGE_BITS); >>>>>>> + if (BITMAP_SIZE(last_ram_offset + size) !=3D >>>>>>> BITMAP_SIZE(last_ram_offset)) { >>>>>> >>>>>> This check is unneeded - the code will work fine even if the bitmap >>>>>> size >>>>>> doesn't change. >>>>> >>>>> OK. I'll remove it. >>>> >>>> I have a problem here. >>>> If I remove this check, glibc reports an error as below. >>>> >>>> *** glibc detected *** /usr/local/qemu/bin/qemu-system-x86_64: >>>> realloc(): invalid pointer: 0x0000000001f0e450 *** >>>> =3D=3D=3D=3D=3D=3D=3D Backtrace: =3D=3D=3D=3D=3D=3D=3D=3D=3D >>>> /lib64/libc.so.6[0x369fa75a96] >>>> /lib64/libc.so.6(realloc+0x2a1)[0x369fa7b881] >>>> /usr/local/qemu/bin/qemu-system-x86_64[0x437d93] >>>> /usr/local/qemu/bin/qemu-system-x86_64[0x4f03f6] >>>> /usr/local/qemu/bin/qemu-system-x86_64[0x5b052c] >>>> /usr/local/qemu/bin/qemu-system-x86_64[0x5b0d8b] >>>> /usr/local/qemu/bin/qemu-system-x86_64[0x41ec2b] >>>> /lib64/libc.so.6(__libc_start_main+0xfd)[0x369fa1ea2d] >>>> /usr/local/qemu/bin/qemu-system-x86_64[0x406479] >>>> =3D=3D=3D=3D=3D=3D=3D Memory map: =3D=3D=3D=3D=3D=3D=3D=3D >>>> >>>> I reminded that I put this check to avoid reallocating same size to >>>> the bitmap. >>>> qemu goes this routine at start up, and extends last_ram_offset at >>>> small numbers. >>>> The error above is reported at the extension phase. >>>> >>> >>> This probably means that an old bitmap pointer leaked somewhere, and we >>> realloc() it after free? Or perhaps a glibc bug. >> >> Original qemu doesn't have a code the frees phys_ram_dirty, and I didn't >> either. >> Hmmm. > > I meant, after we realloc() something we keep using the old pointer. > =A0realloc() is equivalent to free(), after all. > > It might also be memory corruption - bits set outside the bitmap and hitt= ing > glibc malloc metadata. The latter seems to be the problem. I was calling memset() too aggressively when BITMAP_SIZE didn't grow. I think It should be as following. memset((uint8_t *)phys_ram_dirty[i] + BITMAP_SIZE(last_ram_offset), 0xff, BITMAP_SIZE(last_ram_offset + size) - BITMAP_SIZE(last_ram_offset));