Re: [PATCH] ppc: remove excessive logging

[David Gibson]

On Sun, Dec 15, 2019 at 11:59:22AM +0100, BALATON Zoltan wrote:
> On Sun, 15 Dec 2019, Philippe Mathieu-Daudé wrote:
> > Hi Joakim,
> > 
> > I'm cc'ing the PPC maintainers for you, so they won't miss your patch
> > (see
> > https://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer
> > and the output of ./scripts/get_maintainer.pl -f
> > target/ppc/excp_helper.c).
> > 
> > On 12/14/19 1:13 PM, Joakim Tjernlund wrote:
> > > From: Joakim Tjernlund <address@hidden>
> > > 
> > > ppc logs every type of Invalid instruction. This generates a lot
> > > of garbage on console when sshd/ssh_keygen executes as
> > > they try various insn to optimize its performance.
> > > The invalid operation log is still there so an unknown insn
> > > will still be logged.
> > > 
> > > Signed-off-by: Joakim Tjernlund <address@hidden>
> > > ---
> > > 
> > >   As far as I can see, ppc is the only one emiting thsi
> > >   debug msg for Invalid insns.
> > > 
> > > target/ppc/excp_helper.c | 1 -
> > >   1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > > index 50b004d00d..45c2fa3ff9 100644
> > > --- a/target/ppc/excp_helper.c
> > > +++ b/target/ppc/excp_helper.c
> > > @@ -326,7 +326,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu,
> > > int excp_model, int excp)
> > >               env->spr[SPR_BOOKE_ESR] = ESR_FP;
> > >               break;
> > >           case POWERPC_EXCP_INVAL:
> > > -            LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n",
> > > env->nip);
> > 
> > I don't think we want to remove a such important log. Since it make
> > sense to not disturb the console, maybe we can replace some of the
> > LOG_EXCP() calls by qemu_log_mask(LOG_GUEST_ERROR,...) instead?
> 
> I don't think that's a good idea. That would flood the -d guest_errors log
> with unwanted messages that are normal not really due to guest errors. The
> LOG_EXCP() is not enabled by default, you have to edit source to enable it
> so you can also then edit the unwanted messages as well (like commenting
> this one out if you don't like it). If this is removed, invalid opcodes are
> still logged from translate.c but the exception generated for them is not
> logged. I don't know if that's useful or not but these are debug logs so
> depends on what do you want to debug. I don't mind this being removed but
> would be also happy leaving it as it is as it's only shown for developers
> who enable it and might help debugging. Or maybe these could be converted to
> traces (although I generally find traces to be less convenient to work with
> than debug logs and not sure about potential performance impact). So my
> preferences would be in order: 1. leave it alone, 2. remove it, 3. convert
> to traces.

Yes, that's my preference as well.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature