From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48385) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RrUg9-00056k-IL for qemu-devel@nongnu.org; Sun, 29 Jan 2012 08:20:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RrUg7-0008Pm-Fg for qemu-devel@nongnu.org; Sun, 29 Jan 2012 08:20:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:63473) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RrUg7-0008Pi-3b for qemu-devel@nongnu.org; Sun, 29 Jan 2012 08:20:19 -0500 Message-ID: <4F254786.6070903@redhat.com> Date: Sun, 29 Jan 2012 15:20:06 +0200 From: Avi Kivity MIME-Version: 1.0 References: <4F2536B9.4070305@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] exec-obsolete: fix length handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel , Stefan Berger On 01/29/2012 03:16 PM, Blue Swirl wrote: > On Sun, Jan 29, 2012 at 12:08, Avi Kivity wrote: > > On 01/28/2012 08:13 PM, Blue Swirl wrote: > >> Fix suspend/resume broken by off-by-one error in > >> 59abb06198ee9471e29c970f294eae80c0b39be1. > >> > >> Adjust the loop so that it handles correctly the case > >> start = (ram_addr_t)-TARGET_PAGE_SIZE, length = TARGET_PAGE_SIZE. > >> > >> Reported-by: Stefan Berger > >> Signed-off-by: Blue Swirl > >> --- > >> exec-obsolete.h | 10 ++++------ > >> 1 files changed, 4 insertions(+), 6 deletions(-) > >> > >> diff --git a/exec-obsolete.h b/exec-obsolete.h > >> index 03cf35e..1bba970 100644 > >> --- a/exec-obsolete.h > >> +++ b/exec-obsolete.h > >> @@ -81,11 +81,10 @@ static inline void > >> cpu_physical_memory_set_dirty_range(ram_addr_t start, > >> int dirty_flags) > >> { > >> uint8_t *p; > >> - ram_addr_t addr, end; > >> + ram_addr_t cur; > >> > >> - end = start + length; > >> p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS); > >> - for (addr = start; addr <= end; addr += TARGET_PAGE_SIZE) { > >> + for (cur = 0; cur < length; cur += TARGET_PAGE_SIZE) { > >> *p++ |= dirty_flags; > >> } > > > > I think this is still wrong - if length == 2 it will iterate once, but > > we need two iterations if start == 0xfff. > > Yes, tricky. We could do something like > for (cur = start & TARGET_PAGE_MASK; cur < length; cur += TARGET_PAGE_SIZE) { > but I'll send a new patch with just s/<=/> TARGET_PAGE_BITS); for (addr = start; addr <= end; addr += TARGET_PAGE_SIZE) { *p++ |= dirty_flags; @@ -98,7 +99,8 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start, uint8_t *p; ram_addr_t addr, end; - end = start + length; + end = (start + length - 1) | (TARGET_PAGE_SIZE - 1); + start &= TARGET_PAGE_MASK; mask = ~dirty_flags; p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS); for (addr = start; addr <= end; addr += TARGET_PAGE_SIZE) { And a non-terminating migration - not sure if this is the cause. -- error compiling committee.c: too many arguments to function