Re: [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs

From: Branden Bonaby
Date: Wed Aug 21 2019 - 23:35:47 EST


> > +What: /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
> > +Date: August 2019
> > +KernelVersion: 5.3
> > +Contact: Branden Bonaby <brandonbonaby94@xxxxxxxxx>
> > +Description: Fuzz testing status of a vmbus device, whether its in an ON
> > + state or a OFF state
>
> Document what values are actually returned?
>
> > +Users: Debugging tools
> > +
> > +What: /sys/kernel/debug/hyperv/<UUID>/delay/fuzz_test_buffer_interrupt_delay
> > +Date: August 2019
> > +KernelVersion: 5.3
> > +Contact: Branden Bonaby <brandonbonaby94@xxxxxxxxx>
> > +Description: Fuzz testing buffer delay value between 0 - 1000
>
> It would be helpful to document the units -- I think this is 0 to 1000
> microseconds.

you're right, that makes sense I'll add that information in. Also
to confirm, it is microseconds like you said.

> > +static int hv_debugfs_delay_set(void *data, u64 val)
> > +{
> > + if (val >= 1 && val <= 1000)
> > + *(u32 *)data = val;
> > + /*Best to not use else statement here since we want
> > + * the delay to remain the same if val > 1000
> > + */
>
> The standard multi-line comment style would be:
>
> /*
> * Best to not use else statement here since we want
> * the delay to remain the same if val > 1000
> */
>

will change

> > + else if (val <= 0)
> > + *(u32 *)data = 0;
>
> You could consider returning an error for an invalid
> value (< 0, or > 1000).
>

its subtle but it does make sense and shows anyone
reading that the only acceptable values in the
function are 0 <= 1000 at a glance. I'll add
that in.