Re: [Qemu-devel] [PATCH for-2.5] tcg: Increase the highwater reservation

From: Aurelien Jarno <aurelien@aurel32.net>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Developers <qemu-devel@nongnu.org>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH for-2.5] tcg: Increase the highwater reservation
Date: Tue, 1 Dec 2015 17:40:54 +0100	[thread overview]
Message-ID: <20151201164054.GD9577@aurel32.net> (raw)
In-Reply-To: <20151201163451.GC9577@aurel32.net>

On 2015-12-01 17:34, Aurelien Jarno wrote:
> On 2015-12-01 16:28, Peter Maydell wrote:
> > On 1 December 2015 at 16:19, Richard Henderson <rth@twiddle.net> wrote:
> > > If there are a lot of guest memory ops in the TB, the amount of
> > > code generated by tcg_out_tb_finalize could be well more than 1k.
> > > In the short term, increase the reservation larger than any TB
> > > seen in practice.
> > >
> > > Reported-by: Aurelien Jarno <aurelien@aurel32.net>
> > > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > > ---
> > >
> > > Reported and discussed with Aurelien on IRC yesterday.  This seems
> > > to be the easiest fix for the upcoming release.  I will fix this
> > > properly (by modifying every backend's finalize routines) for 2.6.
> > 
> > What would be the result of our hitting this bug? I ask because
> > there's a report on qemu-discuss about a qemu-i386-on-ARM-host
> > bug: http://lists.nongnu.org/archive/html/qemu-discuss/2015-11/msg00042.html
> > and the debug log (http://www.mediafire.com/download/ge611be9vbebbw7/qemu.log)
> > suggests we're segfaulting in translation on the TB shortly
> > after we (successfully) translate a TB whose final 'out' size
> > is 1100 and which has 64 guest writes in it. So I'm wondering
> > if that's actually the same bug this is fixing...
> 
> I don't think this is the same bug. The problem happens because the slow
> path of the softmmu load/store access is written at the end of the TB.
> In user mode, there is no slow path, so nothing is written at the end.

That said the problem reported is likely fixed by this commit that went
just after it has been reported:

commit 644da9b39e477caa80bab69d2847dfcb468f0d33
Author: John Clarke <johnc@kirriwa.net>
Date:   Thu Nov 19 10:30:50 2015 +0100

    tcg: Fix highwater check

    A simple typo in the variable to use when comparing vs the highwater mark.
    Reports are that qemu can in fact segfault occasionally due to this mistake.

    Signed-off-by: John Clarke <johnc@kirriwa.net>
    Signed-off-by: Richard Henderson <rth@twiddle.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net