Re: [RFC PATCH] Adding support for coverage informations

[Konrad Rzeszutek Wilk <konrad@kernel.org>]

On Tue, Jan 29, 2013 at 12:02:55PM +0000, Frediano Ziglio wrote:
> On Tue, 2013-01-29 at 10:56 +0000, Ian Campbell wrote:
> > On Mon, 2013-01-28 at 21:16 +0000, Frediano Ziglio wrote:
> > > From: Frediano Ziglio <frediano.ziglio@citrix.com>
> > > 
> > > This patch introduce coverage support to Xen.
> > > Currently it allows to compile Xen with coverage support but there is no way
> > > to extract them.
> > > 
> > > The declarations came from Linux source files (as you can see from file
> > > headers).
> > > 
> > > It also add a new hypercall (can somebody reserve a number for this stuff?).
> > 
> > You can simply patch xen/include/public/xen.h to declare the new
> > __HYPERVISOR_foo_op. (which I now see you have done in this patch!). If
> > you just want to reserve the number it is also allowed to send just that
> > hunk to reserve a number pending the implementation.
> > 
> > BTW I'd suggest leaving the stub implementation out of the patch until
> > you've decided what it will look like, so it returns ENOSYS instead of
> > EINVAL (or change your stub to return ENOSYS).
> > 
> 
> Easy to change. Actually I discovered that my patch does not even
> compile if you disable TEST_COVERAGE (and it should be disabled by
> default too). EINVAL say that the hypercall is present but the value
> passed in invalid.
> 
> The reason why adding a new hypercall instead of a new sysctl is simply
> because is easier to have a zero cost if you disable coverage
> informations. The best thing would be redirect do_coverage_op to
> do_ni_hypercall using linker options but even two small stub would do
> (these stubs will return ENOSYS instead).

I am not sure I follow. Is the sysctl hypercall code path "longer" than
the hypercall path you are introducing? What is the zero cost?