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

2019-08-21 Thread Branden Bonaby
> > +What:   /sys/kernel/debug/hyperv//fuzz_test_state
> > +Date:   August 2019
> > +KernelVersion:  5.3
> > +Contact:Branden Bonaby 
> > +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//delay/fuzz_test_buffer_interrupt_delay
> > +Date:   August 2019
> > +KernelVersion:  5.3
> > +Contact:Branden Bonaby 
> > +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.



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

2019-08-21 Thread Michael Kelley
From: Branden Bonaby  Sent: Tuesday, August 20, 2019 
4:39 PM
> 
> Expose the test parameters as part of the debugfs channel attributes.
> We will control the testing state via these attributes.
> 
> Signed-off-by: Branden Bonaby 
> ---
> Changes in v3:
>  - Change call to IS_ERR_OR_NULL, to IS_ERR.
> 
> Changes in v2:
>  - Move test attributes to debugfs.
>  - Wrap test code under #ifdef statements.
>  - Add new documentation file under Documentation/ABI/testing.
>  - Make commit message reflect the change from from sysfs to debugfs.
> 
>  Documentation/ABI/testing/debugfs-hyperv |  21 +++
>  MAINTAINERS  |   1 +
>  drivers/hv/vmbus_drv.c   | 167 +++
>  3 files changed, 189 insertions(+)
>  create mode 100644 Documentation/ABI/testing/debugfs-hyperv
> 
> diff --git a/Documentation/ABI/testing/debugfs-hyperv
> b/Documentation/ABI/testing/debugfs-hyperv
> new file mode 100644
> index ..b25f751fafa8
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-hyperv
> @@ -0,0 +1,21 @@
> +What:   /sys/kernel/debug/hyperv//fuzz_test_state
> +Date:   August 2019
> +KernelVersion:  5.3
> +Contact:Branden Bonaby 
> +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//delay/fuzz_test_buffer_interrupt_delay
> +Date:   August 2019
> +KernelVersion:  5.3
> +Contact:Branden Bonaby 
> +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.

> +Users:  Debugging tools
> +
> +What:   /sys/kernel/debug/hyperv//delay/fuzz_test_message_delay
> +Date:   August 2019
> +KernelVersion:  5.3
> +Contact:Branden Bonaby 
> +Description:Fuzz testing message delay value between 0 - 1000

Same here.

> +Users:  Debugging tools
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e81e60bd7c26..120284a8185f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7460,6 +7460,7 @@ F:  include/uapi/linux/hyperv.h
>  F:   include/asm-generic/mshyperv.h
>  F:   tools/hv/
>  F:   Documentation/ABI/stable/sysfs-bus-vmbus
> +F:   Documentation/ABI/testing/debugfs-hyperv
> 
>  HYPERBUS SUPPORT
>  M:   Vignesh Raghavendra 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index ebd35fc35290..d2e47f04d172 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -919,6 +919,10 @@ static void vmbus_device_release(struct device *device)
>   struct hv_device *hv_dev = device_to_hv_device(device);
>   struct vmbus_channel *channel = hv_dev->channel;
> 
> +#ifdef CONFIG_HYPERV_TESTING
> + hv_debug_rm_dev_dir(hv_dev);
> +#endif /* CONFIG_HYPERV_TESTING */

Same comment in as previous patch about #ifdef inline in the code,
and similarly for other occurrences in this patch.

> +
>   mutex_lock(_connection.channel_mutex);
>   hv_process_channel_removal(channel);
>   mutex_unlock(_connection.channel_mutex);
> @@ -1727,6 +1731,9 @@ int vmbus_device_register(struct hv_device 
> *child_device_obj)
>   pr_err("Unable to register primary channeln");
>   goto err_kset_unregister;
>   }
> +#ifdef CONFIG_HYPERV_TESTING
> + hv_debug_add_dev_dir(child_device_obj);
> +#endif /* CONFIG_HYPERV_TESTING */
> 
>   return 0;
> 
> @@ -2086,6 +2093,159 @@ static void hv_crash_handler(struct pt_regs *regs)
>   hyperv_cleanup();
>  };
> 
> +#ifdef CONFIG_HYPERV_TESTING
> +
> +struct dentry *hv_root;
> +
> +static int hv_debugfs_delay_get(void *data, u64 *val)
> +{
> + *val = *(u32 *)data;
> + return 0;
> +}
> +
> +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
 */

> + else if (val <= 0)
> + *(u32 *)data = 0;

You could consider returning an error for an invalid
value (< 0, or > 1000).

> + return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(hv_debugfs_delay_fops, hv_debugfs_delay_get,
> +  hv_debugfs_delay_set, "%llu\n");
> +

Michael


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

2019-08-20 Thread Branden Bonaby
Expose the test parameters as part of the debugfs channel attributes.
We will control the testing state via these attributes.

Signed-off-by: Branden Bonaby 
---
Changes in v3:
 - Change call to IS_ERR_OR_NULL, to IS_ERR.

Changes in v2:
 - Move test attributes to debugfs.
 - Wrap test code under #ifdef statements.
 - Add new documentation file under Documentation/ABI/testing.
 - Make commit message reflect the change from from sysfs to debugfs.

 Documentation/ABI/testing/debugfs-hyperv |  21 +++
 MAINTAINERS  |   1 +
 drivers/hv/vmbus_drv.c   | 167 +++
 3 files changed, 189 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv

diff --git a/Documentation/ABI/testing/debugfs-hyperv 
b/Documentation/ABI/testing/debugfs-hyperv
new file mode 100644
index ..b25f751fafa8
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-hyperv
@@ -0,0 +1,21 @@
+What:   /sys/kernel/debug/hyperv//fuzz_test_state
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing status of a vmbus device, whether its in an ON
+state or a OFF state
+Users:  Debugging tools
+
+What:   
/sys/kernel/debug/hyperv//delay/fuzz_test_buffer_interrupt_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing buffer delay value between 0 - 1000
+Users:  Debugging tools
+
+What:   /sys/kernel/debug/hyperv//delay/fuzz_test_message_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing message delay value between 0 - 1000
+Users:  Debugging tools
diff --git a/MAINTAINERS b/MAINTAINERS
index e81e60bd7c26..120284a8185f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7460,6 +7460,7 @@ F:include/uapi/linux/hyperv.h
 F: include/asm-generic/mshyperv.h
 F: tools/hv/
 F: Documentation/ABI/stable/sysfs-bus-vmbus
+F: Documentation/ABI/testing/debugfs-hyperv
 
 HYPERBUS SUPPORT
 M: Vignesh Raghavendra 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ebd35fc35290..d2e47f04d172 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -919,6 +919,10 @@ static void vmbus_device_release(struct device *device)
struct hv_device *hv_dev = device_to_hv_device(device);
struct vmbus_channel *channel = hv_dev->channel;
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_rm_dev_dir(hv_dev);
+#endif /* CONFIG_HYPERV_TESTING */
+
mutex_lock(_connection.channel_mutex);
hv_process_channel_removal(channel);
mutex_unlock(_connection.channel_mutex);
@@ -1727,6 +1731,9 @@ int vmbus_device_register(struct hv_device 
*child_device_obj)
pr_err("Unable to register primary channeln");
goto err_kset_unregister;
}
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_add_dev_dir(child_device_obj);
+#endif /* CONFIG_HYPERV_TESTING */
 
return 0;
 
@@ -2086,6 +2093,159 @@ static void hv_crash_handler(struct pt_regs *regs)
hyperv_cleanup();
 };
 
+#ifdef CONFIG_HYPERV_TESTING
+
+struct dentry *hv_root;
+
+static int hv_debugfs_delay_get(void *data, u64 *val)
+{
+   *val = *(u32 *)data;
+   return 0;
+}
+
+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
+*/
+   else if (val <= 0)
+   *(u32 *)data = 0;
+   return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(hv_debugfs_delay_fops, hv_debugfs_delay_get,
+hv_debugfs_delay_set, "%llu\n");
+
+/* Setup delay files to store test values */
+int hv_debug_delay_files(struct hv_device *dev, struct dentry *root)
+{
+   struct vmbus_channel *channel = dev->channel;
+   char *buffer = "fuzz_test_buffer_interrupt_delay";
+   char *message = "fuzz_test_message_delay";
+   int *buffer_val = >fuzz_testing_interrupt_delay;
+   int *message_val = >fuzz_testing_message_delay;
+   struct dentry *buffer_file, *message_file;
+
+   buffer_file = debugfs_create_file(buffer, 0644, root,
+ buffer_val,
+ _debugfs_delay_fops);
+   if (IS_ERR(buffer_file)) {
+   pr_debug("debugfs_hyperv: file %s not created\n", buffer);
+   return PTR_ERR(buffer_file);
+   }
+
+   message_file = debugfs_create_file(message, 0644, root,
+  message_val,
+  _debugfs_delay_fops);
+   if (IS_ERR(message_file)) {
+   pr_debug("debugfs_hyperv: file %s not created\n", message);
+   return PTR_ERR(message_file);
+   }
+
+