linux-kernel - Re: [PATCH v6 1/2] perf,kvm/powerpc: Add kvm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150908151027.GM3475@kernel.org>
Date:	Tue, 8 Sep 2015 12:10:27 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Michael Ellerman <mpe@...erman.id.au>
Cc:	Hemant Kumar <hemant@...ux.vnet.ibm.com>,
	Alexander Yarygin <yarygin@...ux.vnet.ibm.com>,
	maddy@...ux.vnet.ibm.com, srikar@...ux.vnet.ibm.com,
	linux-kernel@...r.kernel.org, warrier@...ux.vnet.ibm.com,
	paulus@...ba.org, scottwood@...escale.com,
	sukadev@...ux.vnet.ibm.com, linuxppc-dev@...ts.ozlabs.org,
	mingo@...nel.org
Subject: Re: [PATCH v6 1/2] perf,kvm/powerpc: Add kvm_perf.h for powerpc

Em Tue, Sep 08, 2015 at 06:33:45PM +1000, Michael Ellerman escreveu:
> On Tue, 2015-09-08 at 10:26 +0530, Hemant Kumar wrote:
> > 
> > On 09/07/2015 10:40 AM, Michael Ellerman wrote:
> > > On Fri, 2015-09-04 at 17:51 -0300, Arnaldo Carvalho de Melo wrote:
> > >> Em Tue, Sep 01, 2015 at 12:18:47PM +0530, Hemant Kumar escreveu:
> > >>>> Should I try to process the 5 together, applying thest two first?
> > >>   
> > >>> Yes, this patchset needs to be applied before applying the other patchset,
> > >>> since there is a direct dependency on these two for the tooling part to
> > >>> work.
> > >>   
> > >>>> I see there are no acks from powerpc arch maintainers, how should we
> > >>>> proceed here? If there are no problems with the arch bits, and if it is
> > >>>> just to enable the tooling part, again, should I process the 5 as just
> > >>>> one series?
> > >>   
> > >>> The reason to split the earlier patchset into two was to separate the
> > >>> tooling/perf/ and arch/powerpc/ side patches, as asked by Michael..
> > >>   
> > >>> Here is the link to that discussion :
> > >>> http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg86916.html
> > >>   
> > >>> If Michael is ok with the patches, you can process all the 5 patches
> > >>> together. Michael?
> > >> Michael?
> > > I'm not particularly happy with it.
> > >
> > > Can we at least remove this hunk from the uapi header:
> > >
> > > +/* This is to shut the compiler up */
> > > +#define KVM_ENTRY_TRACE ""
> > > +#define KVM_EXIT_TRACE ""
> > > +#define KVM_EXIT_REASON ""
> > >
> > 
> > Agreed, I didn't like this too, but I kept this because of the generic
> > perf userspace code that looks for KVM_{ENTRY,EXIT}_TRACE and
> > KVM_EXIT_REASON. We can remove this and put this hunk in the
> > userspace side.
> 
> Yes please.
> 
> > Arnaldo,
> > Can we remove the dependency on uapi altogether (also suggested
> > by Scott) because it doesn't seem to fulfill much purpose? Rather,
> > hardcode the events in the userspace completely (since, tracepoint
> > event names are unlikely to change) ? Some of what is being done
> > by x86 already in kvm-stat.c where its defining kvm_events_tp[] and
> > its not using the macros, rather, the tracepoints directly. Macros are
> > only being used in builtin-kvm.c where the tracepoint names are
> > matched with KVM_{ENTRY,EXIT}_TRACE and when we are looking
> > for the key KVM_EXIT_REASON.
> 
> That would certainly make me a lot happier with it.
> 
> Also think about what would happen if the tracepoint names *did* change. The
> perf code would want to try and work with both the old and new names, so at
> that point you'd end up hard coding the names in the perf code anyway.

Yeah, seems like we have a plan :-)

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/