2019-08-20 23:41:08

by Branden Bonaby

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

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 <[email protected]>
---
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 000000000000..b25f751fafa8
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-hyperv
@@ -0,0 +1,21 @@
+What: /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
+Date: August 2019
+KernelVersion: 5.3
+Contact: Branden Bonaby <[email protected]>
+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/<UUID>/delay/fuzz_test_buffer_interrupt_delay
+Date: August 2019
+KernelVersion: 5.3
+Contact: Branden Bonaby <[email protected]>
+Description: Fuzz testing buffer delay value between 0 - 1000
+Users: Debugging tools
+
+What: /sys/kernel/debug/hyperv/<UUID>/delay/fuzz_test_message_delay
+Date: August 2019
+KernelVersion: 5.3
+Contact: Branden Bonaby <[email protected]>
+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 <[email protected]>
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(&vmbus_connection.channel_mutex);
hv_process_channel_removal(channel);
mutex_unlock(&vmbus_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 = &channel->fuzz_testing_interrupt_delay;
+ int *message_val = &channel->fuzz_testing_message_delay;
+ struct dentry *buffer_file, *message_file;
+
+ buffer_file = debugfs_create_file(buffer, 0644, root,
+ buffer_val,
+ &hv_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,
+ &hv_debugfs_delay_fops);
+ if (IS_ERR(message_file)) {
+ pr_debug("debugfs_hyperv: file %s not created\n", message);
+ return PTR_ERR(message_file);
+ }
+
+ return 0;
+}
+
+/* Setup test state value for vmbus device */
+int hv_debug_set_test_state(struct hv_device *dev, struct dentry *root)
+{
+ struct vmbus_channel *channel = dev->channel;
+ bool *state = &channel->fuzz_testing_state;
+ char *status = "fuzz_test_state";
+ struct dentry *test_state;
+
+ test_state = debugfs_create_bool(status, 0644, root, state);
+ if (IS_ERR(test_state)) {
+ pr_debug("debugfs_hyperv: file %s not created\n", status);
+ return PTR_ERR(test_state);
+ }
+
+ return 0;
+}
+
+/* Bind hv device to a dentry for debugfs */
+void hv_debug_set_dir_dentry(struct hv_device *dev, struct dentry *root)
+{
+ if (hv_root)
+ dev->debug_dir = root;
+}
+
+/* Create all test dentry's and names for fuzz testing */
+int hv_debug_add_dev_dir(struct hv_device *dev)
+{
+ const char *device = dev_name(&dev->device);
+ char *delay_name = "delay";
+ struct dentry *delay, *dev_root;
+ int ret;
+
+ if (!IS_ERR(hv_root)) {
+ dev_root = debugfs_create_dir(device, hv_root);
+ if (IS_ERR(dev_root)) {
+ pr_debug("debugfs_hyperv: %s/%s/ not created\n",
+ TESTING, device);
+ return PTR_ERR(dev_root);
+ }
+
+ hv_debug_set_test_state(dev, dev_root);
+ hv_debug_set_dir_dentry(dev, dev_root);
+ delay = debugfs_create_dir(delay_name, dev_root);
+
+ if (IS_ERR(delay)) {
+ pr_debug("debugfs_hyperv: %s/%s/%s/ not created\n",
+ TESTING, device, delay_name);
+ return PTR_ERR(delay);
+ }
+ ret = hv_debug_delay_files(dev, delay);
+
+ return ret;
+ }
+ pr_debug("debugfs_hyperv: %s/ not in root debugfs path\n", TESTING);
+ return PTR_ERR(hv_root);
+}
+
+/* Remove dentry associated with released hv device */
+void hv_debug_rm_dev_dir(struct hv_device *dev)
+{
+ if (!IS_ERR(hv_root))
+ debugfs_remove_recursive(dev->debug_dir);
+}
+
+/* Remove all dentrys associated with vmbus testing */
+void hv_debug_rm_all_dir(void)
+{
+ debugfs_remove_recursive(hv_root);
+}
+
+/* Delay buffer/message reads on a vmbus channel */
+void hv_debug_delay_test(struct vmbus_channel *channel, enum delay delay_type)
+{
+ struct vmbus_channel *test_channel = channel->primary_channel ?
+ channel->primary_channel :
+ channel;
+ bool state = test_channel->fuzz_testing_state;
+
+ if (state) {
+ if (delay_type == 0)
+ udelay(test_channel->fuzz_testing_interrupt_delay);
+ else
+ udelay(test_channel->fuzz_testing_message_delay);
+ }
+}
+
+/* Initialize top dentry for vmbus testing */
+int hv_debug_init(void)
+{
+ hv_root = debugfs_create_dir(TESTING, NULL);
+ if (IS_ERR(hv_root)) {
+ pr_debug("debugfs_hyperv: %s/ not created\n", TESTING);
+ return PTR_ERR(hv_root);
+ }
+
+ return 0;
+}
+#endif /* CONFIG_HYPERV_TESTING */
+
static int __init hv_acpi_init(void)
{
int ret, t;
@@ -2108,6 +2268,9 @@ static int __init hv_acpi_init(void)
ret = -ETIMEDOUT;
goto cleanup;
}
+#ifdef CONFIG_HYPERV_TESTING
+ hv_debug_init();
+#endif /* CONFIG_HYPERV_TESTING */

ret = vmbus_bus_init();
if (ret)
@@ -2140,6 +2303,10 @@ static void __exit vmbus_exit(void)

tasklet_kill(&hv_cpu->msg_dpc);
}
+#ifdef CONFIG_HYPERV_TESTING
+ hv_debug_rm_all_dir();
+#endif /* CONFIG_HYPERV_TESTING */
+
vmbus_free_channels();

if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
--
2.17.1


2019-08-22 00:03:55

by Michael Kelley (LINUX)

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

From: Branden Bonaby <[email protected]> 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 <[email protected]>
> ---
> 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 000000000000..b25f751fafa8
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-hyperv
> @@ -0,0 +1,21 @@
> +What: /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
> +Date: August 2019
> +KernelVersion: 5.3
> +Contact: Branden Bonaby <[email protected]>
> +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 <[email protected]>
> +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/<UUID>/delay/fuzz_test_message_delay
> +Date: August 2019
> +KernelVersion: 5.3
> +Contact: Branden Bonaby <[email protected]>
> +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 <[email protected]>
> 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(&vmbus_connection.channel_mutex);
> hv_process_channel_removal(channel);
> mutex_unlock(&vmbus_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

2019-08-22 03:42:16

by Branden Bonaby

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

> > +What: /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
> > +Date: August 2019
> > +KernelVersion: 5.3
> > +Contact: Branden Bonaby <[email protected]>
> > +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 <[email protected]>
> > +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.