> > endmenu
> > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> > index 09829e15d4a0..c9c63a4033cd 100644
> > --- a/drivers/hv/connection.c
> > +++ b/drivers/hv/connection.c
> > @@ -357,6 +357,9 @@ void vmbus_on_event(unsigned long data)
> >
> > trace_vmbus_on_event(channel);
> >
> > +#ifdef CONFIG_HYPERV_TESTING
> > + hv_debug_delay_test(channel, INTERRUPT_DELAY);
> > +#endif /* CONFIG_HYPERV_TESTING */
>
> You are following Vitaly's suggestion to use #ifdef's so no code is
> generated when HYPERV_TESTING is not enabled. However, this
> direct approach to using #ifdef's really clutters the code and makes
> it harder to read and follow. The better approach is to use the
> #ifdef in the include file where the functions are defined. If
> HYPERV_TESTING is not enabled, provide a #else that defines
> the function with an empty implementation for which the compiler
> will generate no code. An as example, see the function definition
> for hyperv_init() in arch/x86/include/asm/mshyperv.h. There are
> several functions treated similarly in that include file.
>
I checked out the code in arch/x86/include/asm/mshyperv.h, after
thinking about it, I'm wondering if it would be better just to have
two files one called hv_debugfs.c and the other hyperv_debugfs.h.
I could put the code definitions in hv_debugfs.c and at the top
include the hyperv_debugfs.h file which would house the declarations
of these functions under the ifdef. Then like you alluded too use
an #else statement that would have the null implementations of the
above functions. Then put an #include "hyperv_debugfs.h" in the
hyperv_vmbus.h file. I figured instead of putting the code directly
into the vmbus_drv.c file it might be best to put them in a seperate
file like hv_debugfs.c. This way when we start adding more tests we
don't bloat the vmbus_drv.c file unnecessarily. The hv_debugfs.c
file would have the #ifdef CONFIG_HYPERV_TESTING at the top so if
its not enabled those null implementations in "hyperv_debugfs.h"
woud kick in anywhere that included the hyperv_vmbus.h file which
is what we want.
what do you think?
>
> > do {
> > void (*callback_fn)(void *);
> >
> > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> > index 362e70e9d145..edf14f596d8c 100644
> > --- a/drivers/hv/hyperv_vmbus.h
> > +++ b/drivers/hv/hyperv_vmbus.h
> > @@ -357,4 +357,24 @@ enum hvutil_device_state {
> > HVUTIL_DEVICE_DYING, /* driver unload is in progress */
> > };
> >
> > +#ifdef CONFIG_HYPERV_TESTING
> > +#include <linux/debugfs.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
>
> Generally #include files should go at the top of the file, even if they
> are only needed conditionally.
>
I see , will change
> > +#define TESTING "hyperv"
>
> I'm not seeing what this line is for, or how it is used.
I used it as the top level name for the dentry that
would appear in debugfs but now I realize its actually
not needed, so i'll remove this.
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -926,6 +926,21 @@ struct vmbus_channel {
> > * full outbound ring buffer.
> > */
> > u64 out_full_first;
> > +
> > +#ifdef CONFIG_HYPERV_TESTING
> > + /* enabling/disabling fuzz testing on the channel (default is false)*/
> > + bool fuzz_testing_state;
> > +
> > + /* Interrupt delay will delay the guest from emptying the ring buffer
> > + * for a specific amount of time. The delay is in microseconds and will
> > + * be between 1 to a maximum of 1000, its default is 0 (no delay).
> > + * The Message delay will delay guest reading on a per message basis
> > + * in microseconds between 1 to 1000 with the default being 0
> > + * (no delay).
> > + */
> > + u32 fuzz_testing_interrupt_delay;
> > + u32 fuzz_testing_message_delay;
> > +#endif /* CONFIG_HYPERV_TESTING */
>
> For fields in a data structure like this, you don't have much choice
> but to put the #ifdef directly inline. However, for small fields like this
> and where the data structure isn't size sensitive, you could consider
> omitting the #ifdef and just always including the fields even when
> HYPERV_TESTING is not enabled. I don't have a strong preference
> either way.
>
I'll take the ifdefs out since the fields aren't too big
