Re: [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs
> > +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
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
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); + } + +