Re: [PATCH 2/3] migration: Drop unnecessary check in ram's pending_exact()

[Nina Schoetterl-Glausch <nsg@linux.ibm.com>]

On Wed, 2024-03-20 at 14:57 -0400, Peter Xu wrote:
> On Wed, Mar 20, 2024 at 06:51:26PM +0100, Nina Schoetterl-Glausch wrote:
> > On Wed, 2024-01-17 at 15:58 +0800, peterx@redhat.com wrote:
> > > From: Peter Xu <peterx@redhat.com>
> > > 
> > > When the migration frameworks fetches the exact pending sizes, it means
> > > this check:
> > > 
> > >   remaining_size < s->threshold_size
> > > 
> > > Must have been done already, actually at migration_iteration_run():
> > > 
> > >     if (must_precopy <= s->threshold_size) {
> > >         qemu_savevm_state_pending_exact(&must_precopy, &can_postcopy);
> > > 
> > > That should be after one round of ram_state_pending_estimate().  It makes
> > > the 2nd check meaningless and can be dropped.
> > > 
> > > To say it in another way, when reaching ->state_pending_exact(), we
> > > unconditionally sync dirty bits for precopy.
> > > 
> > > Then we can drop migrate_get_current() there too.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > 
> > Hi Peter,
> 
> Hi, Nina,
> 
> > 
> > could you have a look at this issue:
> > https://gitlab.com/qemu-project/qemu/-/issues/1565
> > 
> > which I reopened. Previous thread here:
> > 
> > https://lore.kernel.org/qemu-devel/20230324184129.3119575-1-nsg@linux.ibm.com/
> > 
> > I'm seeing migration failures with s390x TCG again, which look the same to me
> > as those a while back.
> 
> I'm still quite confused how that could be caused of this.
> 
> What you described in the previous bug report seems to imply some page was
> leftover in migration so some page got corrupted after migrated.
> 
> However what this patch mostly does is it can sync more than before even if
> I overlooked the condition check there (I still think the check is
> redundant, there's one outlier when remaining_size == threshold_size, but I
> don't think it should matter here as of now).  It'll make more sense if
> this patch made the sync less, but that's not the case but vice versa.

[...]

> In the previous discussion, you mentioned that you bisected to the commit
> and also verified the fix.  Now you also mentioned in the bz that you can't
> reporduce this bug manually.
> 
> Is it still possible to be reproduced with some scripts?  Do you also mean
> that it's harder to reproduce comparing to before?  In all cases, some way
> to reproduce it would definitely be helpful.

I tried running the kvm-unit-test a bunch of times in a loop and couldn't
trigger a failure. I just tried again on a different system and managed just
fine, yay. No idea why it wouldn't on the first system tho.
> 
> Even if we want to revert this change, we'll need to know whether this will
> fix your case so we need something to verify it before a revert.  I'll
> consider that the last though as I had a feeling this is papering over
> something else.

I can check if I can reproduce the issue before & after b0504edd ("migration:
Drop unnecessary check in ram's pending_exact()").
I can also check if I can reproduce it on x86, that worked last time.
Anything else? Ideas on how to pinpoint where the corruption happens?

> 
> Thanks,
>