All of lore.kernel.org
 help / color / mirror / Atom feed
From: Branden Bonaby <brandonbonaby94@gmail.com>
To: Michael Kelley <mikelley@microsoft.com>
Cc: KY Srinivasan <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Stephen Hemminger <sthemmin@microsoft.com>,
	"sashal@kernel.org" <sashal@kernel.org>,
	"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs
Date: Wed, 21 Aug 2019 23:36:09 -0400	[thread overview]
Message-ID: <20190822033609.GB37262@Test-Virtual-Machine> (raw)
In-Reply-To: <DM5PR21MB0137B4071E64688C5F902E83D7AA0@DM5PR21MB0137.namprd21.prod.outlook.com>

> > +What:           /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
> > +Date:           August 2019
> > +KernelVersion:  5.3
> > +Contact:        Branden Bonaby <brandonbonaby94@gmail.com>
> > +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@gmail.com>
> > +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.


  reply	other threads:[~2019-08-22  3:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-20 23:38 [PATCH v3 0/3] hv: vmbus: add fuzz testing to hv device Branden Bonaby
2019-08-20 23:39 ` [PATCH v3 1/3] drivers: hv: vmbus: Introduce latency testing Branden Bonaby
2019-08-29 21:57   ` Stephen Hemminger
2019-09-06  0:20     ` Branden Bonaby
2019-08-20 23:39 ` [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs Branden Bonaby
2019-08-21 23:10   ` Michael Kelley
2019-08-22  3:36     ` Branden Bonaby [this message]
2019-08-20 23:40 ` [PATCH v3 3/3] tools: hv: add vmbus testing tool Branden Bonaby
2019-08-22  1:36   ` Harry Zhang
2019-08-22  3:16     ` Branden Bonaby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190822033609.GB37262@Test-Virtual-Machine \
    --to=brandonbonaby94@gmail.com \
    --cc=haiyangz@microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikelley@microsoft.com \
    --cc=sashal@kernel.org \
    --cc=sthemmin@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.