Re: [RFC PATCH] Adding support for coverage informations

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

On Fri, Feb 01, 2013 at 03:05:23PM +0000, Frediano Ziglio wrote:
> On Fri, 2013-02-01 at 09:46 -0500, Konrad Rzeszutek Wilk wrote:
> > On Fri, Feb 01, 2013 at 02:29:04PM +0000, Frediano Ziglio wrote:
> > > On Thu, 2013-01-31 at 08:51 +0000, Ian Campbell wrote:
> > > > On Wed, 2013-01-30 at 21:34 +0000, Konrad Rzeszutek Wilk wrote:
> > > > > > 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?
> > > > 
> > > > I don't think we care a jot about the performance of this system call
> > > > when coverage is disabled, it's certainly not a hot path and in any case
> > > > if it is a NOP it doesn't really matter anyway.
> > > > 
> > > > Ian.
> > > > 
> > > 
> > > It's not about the speed, it's about the bytes introduced in Xen binary.
> > 
> > Not sure I follow. What bytes? Just that the code size is bigger b/c it
> > will go through the sysctl functions? How many bytes of extra code are
> > talking here? 
> 
> Probably less than 20...
> 
> In do_sysctl something like
> 
>     case XEN_SYSCTL_coverage_op:
>         ret = coverage_op(&op->u.coverage_op);
>         break;
> 
> where when disabled coverage_op should be declared as
> 
> static inline long coverage_op(struct xen_sysvtl_coverage_op *op)
> {
>     return -ENOSYS;
> }

Ok. Then it looks like XEN_SYSCTL_* it is the right place.