Re: [PATCH 6/8] ntb_tool: Add link status file to debugfs

Re: [PATCH 6/8] ntb_tool: Add link status file to debugfs

[Allen Hubbe]

On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> In order to more successfully script with ntb_tool it's useful to
> have a link file to check the link status so that the script
> doesn't use the other files until the link is up.
>
> This commit adds a 'link' file to the debugfs directory which reads
> 0 or 1 depending on the link status. For scripting convenience, writing
> will block until the link is up (discarding anything that was written).
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/ntb/test/ntb_tool.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index 954e1d5..116352e 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -59,6 +59,12 @@
>   *
>   * Eg: check if clearing the doorbell mask generates an interrupt.
>   *
> + * # Check the link status
> + * root@self# cat $DBG_DIR/link
> + *
> + * # Block until the link is up
> + * root@self# echo > $DBG_DIR/link

I think a file to get and set the link status is a good idea, but the
way it is done as proposed here is not in a similar style to other
ntb_tool operations.  Other operations simply read a register and
format the value, or scan a value and write a register.  Similarly, I
think the link status could be done in the same way: use the read file
operation to get the current status with ntb_link_is_up(), and use the
file write operation to enable or disable the link with
ntb_link_enable() and ntb_link_disable().

Waiting for link status is an interesting concept, too.  Really, one
might be interested in a change in link status, whether up or down.
What about a link event file that supports write to arm the event, and
read to block for the event.  Consider an implementation based on
<linux/completion.h>.  It would be used in combination with the link
status file, above, as follows.

1: Write 1 to the event file.  This arms the event.
  - The event will be disarmed by the next tool_link_event().

2: The application may read the link status file if it is interested
in waiting for a particular event.

3. The application may wait for an event by reading the event file
  - The application will wait as long as the event is still armed.
  - If the event was disarmed before waiting, the application will not block.

4. The application should read the link status again.

In any case, I think it would be more expected and natural to block
while reading a file versus writing it.

> + *
>   * # Set the doorbell mask
>   * root@self# echo 's 1' > $DBG_DIR/mask
>   *
> @@ -127,6 +133,7 @@ struct tool_ctx {
>         struct work_struct link_cleanup;
>         bool link_is_up;
>         struct delayed_work link_work;
> +       wait_queue_head_t link_wq;
>         int mw_count;
>         struct tool_mw mws[MAX_MWS];
>  };
> @@ -237,6 +244,7 @@ static void tool_link_work(struct work_struct *work)
>                         "Error setting up memory windows: %d\n", rc);
>
>         tc->link_is_up = true;
> +       wake_up(&tc->link_wq);
>  }
>
>  static void tool_link_cleanup(struct work_struct *work)
> @@ -573,6 +581,39 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
>                       tool_peer_spad_read,
>                       tool_peer_spad_write);
>
> +static ssize_t tool_link_read(struct file *filep, char __user *ubuf,
> +                             size_t size, loff_t *offp)
> +{
> +       struct tool_ctx *tc = filep->private_data;
> +       char *buf;
> +       ssize_t pos, rc;
> +
> +       buf = kmalloc(64, GFP_KERNEL);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       pos = scnprintf(buf, 64, "%d\n", tc->link_is_up);
> +       rc = simple_read_from_buffer(ubuf, size, offp, buf, pos);
> +
> +       kfree(buf);
> +
> +       return rc;
> +}
> +
> +static ssize_t tool_link_write(struct file *filep, const char __user *ubuf,
> +                              size_t size, loff_t *offp)
> +{
> +       struct tool_ctx *tc = filep->private_data;
> +
> +       if (wait_event_interruptible(tc->link_wq, tc->link_is_up))
> +               return -ERESTART;
> +
> +       return size;
> +}
> +
> +static TOOL_FOPS_RDWR(tool_link_fops,
> +                     tool_link_read,
> +                     tool_link_write);
>
>  static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
>                             size_t size, loff_t *offp)
> @@ -708,6 +749,9 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
>         debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
>                             tc, &tool_peer_spad_fops);
>
> +       debugfs_create_file("link", S_IRUSR | S_IWUSR, tc->dbgfs,
> +                           tc, &tool_link_fops);
> +
>         mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
>         for (i = 0; i < mw_count; i++) {
>                 char buf[30];
> @@ -741,6 +785,7 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
>         }
>
>         tc->ntb = ntb;
> +       init_waitqueue_head(&tc->link_wq);
>         INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
>         INIT_WORK(&tc->link_cleanup, tool_link_cleanup);

Re: [PATCH 6/8] ntb_tool: Add link status file to debugfs

[Logan Gunthorpe]

Hey Allen,

Thanks for the feedback it's a bit more complicated but I don't object 
to that. I'll work something up on Monday.

I was trying to avoid adding link controls, but if we do, would you say 
the module should still enable the link when it's installed? Or would we 
have the user explicitly have to enable the link before using it?

Thanks,

Logan

On 10/06/16 08:27 PM, Allen Hubbe wrote:
> On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>> In order to more successfully script with ntb_tool it's useful to
>> have a link file to check the link status so that the script
>> doesn't use the other files until the link is up.
>>
>> This commit adds a 'link' file to the debugfs directory which reads
>> 0 or 1 depending on the link status. For scripting convenience, writing
>> will block until the link is up (discarding anything that was written).
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> ---
>>   drivers/ntb/test/ntb_tool.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>
>> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
>> index 954e1d5..116352e 100644
>> --- a/drivers/ntb/test/ntb_tool.c
>> +++ b/drivers/ntb/test/ntb_tool.c
>> @@ -59,6 +59,12 @@
>>    *
>>    * Eg: check if clearing the doorbell mask generates an interrupt.
>>    *
>> + * # Check the link status
>> + * root@self# cat $DBG_DIR/link
>> + *
>> + * # Block until the link is up
>> + * root@self# echo > $DBG_DIR/link
>
> I think a file to get and set the link status is a good idea, but the
> way it is done as proposed here is not in a similar style to other
> ntb_tool operations.  Other operations simply read a register and
> format the value, or scan a value and write a register.  Similarly, I
> think the link status could be done in the same way: use the read file
> operation to get the current status with ntb_link_is_up(), and use the
> file write operation to enable or disable the link with
> ntb_link_enable() and ntb_link_disable().
>
> Waiting for link status is an interesting concept, too.  Really, one
> might be interested in a change in link status, whether up or down.
> What about a link event file that supports write to arm the event, and
> read to block for the event.  Consider an implementation based on
> <linux/completion.h>.  It would be used in combination with the link
> status file, above, as follows.
>
> 1: Write 1 to the event file.  This arms the event.
>    - The event will be disarmed by the next tool_link_event().
>
> 2: The application may read the link status file if it is interested
> in waiting for a particular event.
>
> 3. The application may wait for an event by reading the event file
>    - The application will wait as long as the event is still armed.
>    - If the event was disarmed before waiting, the application will not block.
>
> 4. The application should read the link status again.
>
> In any case, I think it would be more expected and natural to block
> while reading a file versus writing it.
>
>> + *
>>    * # Set the doorbell mask
>>    * root@self# echo 's 1' > $DBG_DIR/mask
>>    *
>> @@ -127,6 +133,7 @@ struct tool_ctx {
>>          struct work_struct link_cleanup;
>>          bool link_is_up;
>>          struct delayed_work link_work;
>> +       wait_queue_head_t link_wq;
>>          int mw_count;
>>          struct tool_mw mws[MAX_MWS];
>>   };
>> @@ -237,6 +244,7 @@ static void tool_link_work(struct work_struct *work)
>>                          "Error setting up memory windows: %d\n", rc);
>>
>>          tc->link_is_up = true;
>> +       wake_up(&tc->link_wq);
>>   }
>>
>>   static void tool_link_cleanup(struct work_struct *work)
>> @@ -573,6 +581,39 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
>>                        tool_peer_spad_read,
>>                        tool_peer_spad_write);
>>
>> +static ssize_t tool_link_read(struct file *filep, char __user *ubuf,
>> +                             size_t size, loff_t *offp)
>> +{
>> +       struct tool_ctx *tc = filep->private_data;
>> +       char *buf;
>> +       ssize_t pos, rc;
>> +
>> +       buf = kmalloc(64, GFP_KERNEL);
>> +       if (!buf)
>> +               return -ENOMEM;
>> +
>> +       pos = scnprintf(buf, 64, "%d\n", tc->link_is_up);
>> +       rc = simple_read_from_buffer(ubuf, size, offp, buf, pos);
>> +
>> +       kfree(buf);
>> +
>> +       return rc;
>> +}
>> +
>> +static ssize_t tool_link_write(struct file *filep, const char __user *ubuf,
>> +                              size_t size, loff_t *offp)
>> +{
>> +       struct tool_ctx *tc = filep->private_data;
>> +
>> +       if (wait_event_interruptible(tc->link_wq, tc->link_is_up))
>> +               return -ERESTART;
>> +
>> +       return size;
>> +}
>> +
>> +static TOOL_FOPS_RDWR(tool_link_fops,
>> +                     tool_link_read,
>> +                     tool_link_write);
>>
>>   static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
>>                              size_t size, loff_t *offp)
>> @@ -708,6 +749,9 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
>>          debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
>>                              tc, &tool_peer_spad_fops);
>>
>> +       debugfs_create_file("link", S_IRUSR | S_IWUSR, tc->dbgfs,
>> +                           tc, &tool_link_fops);
>> +
>>          mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
>>          for (i = 0; i < mw_count; i++) {
>>                  char buf[30];
>> @@ -741,6 +785,7 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
>>          }
>>
>>          tc->ntb = ntb;
>> +       init_waitqueue_head(&tc->link_wq);
>>          INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
>>          INIT_WORK(&tc->link_cleanup, tool_link_cleanup);

Re: [PATCH 6/8] ntb_tool: Add link status file to debugfs

[Allen Hubbe]

On Sat, Jun 11, 2016 at 11:28 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
> Hey Allen,
>
> Thanks for the feedback it's a bit more complicated but I don't object to
> that. I'll work something up on Monday.
>
> I was trying to avoid adding link controls, but if we do, would you say the
> module should still enable the link when it's installed? Or would we have
> the user explicitly have to enable the link before using it?

I would vote to keep the current behavior and enable the link when the
module loads.

>
> Thanks,
>
> Logan
>
>
> On 10/06/16 08:27 PM, Allen Hubbe wrote:
>>
>> On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com>
>> wrote:
>>>
>>> In order to more successfully script with ntb_tool it's useful to
>>> have a link file to check the link status so that the script
>>> doesn't use the other files until the link is up.
>>>
>>> This commit adds a 'link' file to the debugfs directory which reads
>>> 0 or 1 depending on the link status. For scripting convenience, writing
>>> will block until the link is up (discarding anything that was written).
>>>
>>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>>> ---
>>>   drivers/ntb/test/ntb_tool.c | 45
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 45 insertions(+)
>>>
>>> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
>>> index 954e1d5..116352e 100644
>>> --- a/drivers/ntb/test/ntb_tool.c
>>> +++ b/drivers/ntb/test/ntb_tool.c
>>> @@ -59,6 +59,12 @@
>>>    *
>>>    * Eg: check if clearing the doorbell mask generates an interrupt.
>>>    *
>>> + * # Check the link status
>>> + * root@self# cat $DBG_DIR/link
>>> + *
>>> + * # Block until the link is up
>>> + * root@self# echo > $DBG_DIR/link
>>
>>
>> I think a file to get and set the link status is a good idea, but the
>> way it is done as proposed here is not in a similar style to other
>> ntb_tool operations.  Other operations simply read a register and
>> format the value, or scan a value and write a register.  Similarly, I
>> think the link status could be done in the same way: use the read file
>> operation to get the current status with ntb_link_is_up(), and use the
>> file write operation to enable or disable the link with
>> ntb_link_enable() and ntb_link_disable().
>>
>> Waiting for link status is an interesting concept, too.  Really, one
>> might be interested in a change in link status, whether up or down.
>> What about a link event file that supports write to arm the event, and
>> read to block for the event.  Consider an implementation based on
>> <linux/completion.h>.  It would be used in combination with the link
>> status file, above, as follows.
>>
>> 1: Write 1 to the event file.  This arms the event.
>>    - The event will be disarmed by the next tool_link_event().
>>
>> 2: The application may read the link status file if it is interested
>> in waiting for a particular event.
>>
>> 3. The application may wait for an event by reading the event file
>>    - The application will wait as long as the event is still armed.
>>    - If the event was disarmed before waiting, the application will not
>> block.
>>
>> 4. The application should read the link status again.
>>
>> In any case, I think it would be more expected and natural to block
>> while reading a file versus writing it.
>>
>>> + *
>>>    * # Set the doorbell mask
>>>    * root@self# echo 's 1' > $DBG_DIR/mask
>>>    *
>>> @@ -127,6 +133,7 @@ struct tool_ctx {
>>>          struct work_struct link_cleanup;
>>>          bool link_is_up;
>>>          struct delayed_work link_work;
>>> +       wait_queue_head_t link_wq;
>>>          int mw_count;
>>>          struct tool_mw mws[MAX_MWS];
>>>   };
>>> @@ -237,6 +244,7 @@ static void tool_link_work(struct work_struct *work)
>>>                          "Error setting up memory windows: %d\n", rc);
>>>
>>>          tc->link_is_up = true;
>>> +       wake_up(&tc->link_wq);
>>>   }
>>>
>>>   static void tool_link_cleanup(struct work_struct *work)
>>> @@ -573,6 +581,39 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
>>>                        tool_peer_spad_read,
>>>                        tool_peer_spad_write);
>>>
>>> +static ssize_t tool_link_read(struct file *filep, char __user *ubuf,
>>> +                             size_t size, loff_t *offp)
>>> +{
>>> +       struct tool_ctx *tc = filep->private_data;
>>> +       char *buf;
>>> +       ssize_t pos, rc;
>>> +
>>> +       buf = kmalloc(64, GFP_KERNEL);
>>> +       if (!buf)
>>> +               return -ENOMEM;
>>> +
>>> +       pos = scnprintf(buf, 64, "%d\n", tc->link_is_up);
>>> +       rc = simple_read_from_buffer(ubuf, size, offp, buf, pos);
>>> +
>>> +       kfree(buf);
>>> +
>>> +       return rc;
>>> +}
>>> +
>>> +static ssize_t tool_link_write(struct file *filep, const char __user
>>> *ubuf,
>>> +                              size_t size, loff_t *offp)
>>> +{
>>> +       struct tool_ctx *tc = filep->private_data;
>>> +
>>> +       if (wait_event_interruptible(tc->link_wq, tc->link_is_up))
>>> +               return -ERESTART;
>>> +
>>> +       return size;
>>> +}
>>> +
>>> +static TOOL_FOPS_RDWR(tool_link_fops,
>>> +                     tool_link_read,
>>> +                     tool_link_write);
>>>
>>>   static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
>>>                              size_t size, loff_t *offp)
>>> @@ -708,6 +749,9 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
>>>          debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
>>>                              tc, &tool_peer_spad_fops);
>>>
>>> +       debugfs_create_file("link", S_IRUSR | S_IWUSR, tc->dbgfs,
>>> +                           tc, &tool_link_fops);
>>> +
>>>          mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
>>>          for (i = 0; i < mw_count; i++) {
>>>                  char buf[30];
>>> @@ -741,6 +785,7 @@ static int tool_probe(struct ntb_client *self, struct
>>> ntb_dev *ntb)
>>>          }
>>>
>>>          tc->ntb = ntb;
>>> +       init_waitqueue_head(&tc->link_wq);
>>>          INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
>>>          INIT_WORK(&tc->link_cleanup, tool_link_cleanup);

RE: [PATCH 6/8] ntb_tool: Add link status file to debugfs

[Allen Hubbe]

From: Allen Hubbe
> On Sat, Jun 11, 2016 at 11:28 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
> > Hey Allen,
> >
> > Thanks for the feedback it's a bit more complicated but I don't object to
> > that. I'll work something up on Monday.
> >
> > I was trying to avoid adding link controls, but if we do, would you say the
> > module should still enable the link when it's installed? Or would we have
> > the user explicitly have to enable the link before using it?
> 
> I would vote to keep the current behavior and enable the link when the
> module loads.
> 
> >
> > Thanks,
> >
> > Logan
> >
> >
> > On 10/06/16 08:27 PM, Allen Hubbe wrote:
> >>
> >> On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com>
> >> wrote:
> >>>
> >>> In order to more successfully script with ntb_tool it's useful to
> >>> have a link file to check the link status so that the script
> >>> doesn't use the other files until the link is up.
> >>>
> >>> This commit adds a 'link' file to the debugfs directory which reads
> >>> 0 or 1 depending on the link status. For scripting convenience, writing
> >>> will block until the link is up (discarding anything that was written).
> >>>
> >>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> >>> ---
> >>>   drivers/ntb/test/ntb_tool.c | 45
> >>> +++++++++++++++++++++++++++++++++++++++++++++
> >>>   1 file changed, 45 insertions(+)
> >>>
> >>> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> >>> index 954e1d5..116352e 100644
> >>> --- a/drivers/ntb/test/ntb_tool.c
> >>> +++ b/drivers/ntb/test/ntb_tool.c
> >>> @@ -59,6 +59,12 @@
> >>>    *
> >>>    * Eg: check if clearing the doorbell mask generates an interrupt.
> >>>    *
> >>> + * # Check the link status
> >>> + * root@self# cat $DBG_DIR/link
> >>> + *
> >>> + * # Block until the link is up
> >>> + * root@self# echo > $DBG_DIR/link
> >>
> >>
> >> I think a file to get and set the link status is a good idea, but the
> >> way it is done as proposed here is not in a similar style to other
> >> ntb_tool operations.  Other operations simply read a register and
> >> format the value, or scan a value and write a register.  Similarly, I
> >> think the link status could be done in the same way: use the read file
> >> operation to get the current status with ntb_link_is_up(), and use the
> >> file write operation to enable or disable the link with
> >> ntb_link_enable() and ntb_link_disable().
> >>
> >> Waiting for link status is an interesting concept, too.  Really, one
> >> might be interested in a change in link status, whether up or down.
> >> What about a link event file that supports write to arm the event, and
> >> read to block for the event.  Consider an implementation based on
> >> <linux/completion.h>.  It would be used in combination with the link
> >> status file, above, as follows.
> >>
> >> 1: Write 1 to the event file.  This arms the event.
> >>    - The event will be disarmed by the next tool_link_event().
> >>
> >> 2: The application may read the link status file if it is interested
> >> in waiting for a particular event.
> >>
> >> 3. The application may wait for an event by reading the event file
> >>    - The application will wait as long as the event is still armed.
> >>    - If the event was disarmed before waiting, the application will not
> >> block.
> >>
> >> 4. The application should read the link status again.
> >>
> >> In any case, I think it would be more expected and natural to block
> >> while reading a file versus writing it.

Feel free to disregard my suggestion above.  I hope my comment has not cost you too much time.

The way you have written it already, and used it in the self-test script is much more concise.

> > + * root@self# echo > $DBG_DIR/link

Acked-by: Allen.Hubbe@emc.com



Eventually, I think it would be useful to let ntb_tool enable and disable the link.  In that case, it might also be useful in a test script to wait for link down, not just link up.

What about this:

# Wait for the link to be up or down
root@self# echo 1 > $DBG_DIR/link
root@self# echo 0 > $DBG_DIR/link

It need not be a part of this patch, but eventually:

# Enable or disable the link
root@self# echo 1 > $DBG_DIR/link_ctrl
root@self# echo 0 > $DBG_DIR/link_ctrl

# Reading the link_ctrl file can also give the link status
root@self# cat $DBG_DIR/link_ctrl

Finally, I wonder if the file called "link" in this patch should be called "link_wait" or similar, so its purpose is obviously not for enabling and disabling the link.

> >>
> >>> + *
> >>>    * # Set the doorbell mask
> >>>    * root@self# echo 's 1' > $DBG_DIR/mask
> >>>    *
> >>> @@ -127,6 +133,7 @@ struct tool_ctx {
> >>>          struct work_struct link_cleanup;
> >>>          bool link_is_up;
> >>>          struct delayed_work link_work;
> >>> +       wait_queue_head_t link_wq;
> >>>          int mw_count;
> >>>          struct tool_mw mws[MAX_MWS];
> >>>   };
> >>> @@ -237,6 +244,7 @@ static void tool_link_work(struct work_struct *work)
> >>>                          "Error setting up memory windows: %d\n", rc);
> >>>
> >>>          tc->link_is_up = true;
> >>> +       wake_up(&tc->link_wq);
> >>>   }
> >>>
> >>>   static void tool_link_cleanup(struct work_struct *work)
> >>> @@ -573,6 +581,39 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
> >>>                        tool_peer_spad_read,
> >>>                        tool_peer_spad_write);
> >>>
> >>> +static ssize_t tool_link_read(struct file *filep, char __user *ubuf,
> >>> +                             size_t size, loff_t *offp)
> >>> +{
> >>> +       struct tool_ctx *tc = filep->private_data;
> >>> +       char *buf;
> >>> +       ssize_t pos, rc;
> >>> +
> >>> +       buf = kmalloc(64, GFP_KERNEL);
> >>> +       if (!buf)
> >>> +               return -ENOMEM;
> >>> +
> >>> +       pos = scnprintf(buf, 64, "%d\n", tc->link_is_up);
> >>> +       rc = simple_read_from_buffer(ubuf, size, offp, buf, pos);
> >>> +
> >>> +       kfree(buf);
> >>> +
> >>> +       return rc;
> >>> +}
> >>> +
> >>> +static ssize_t tool_link_write(struct file *filep, const char __user
> >>> *ubuf,
> >>> +                              size_t size, loff_t *offp)
> >>> +{
> >>> +       struct tool_ctx *tc = filep->private_data;
> >>> +
> >>> +       if (wait_event_interruptible(tc->link_wq, tc->link_is_up))
> >>> +               return -ERESTART;
> >>> +
> >>> +       return size;
> >>> +}
> >>> +
> >>> +static TOOL_FOPS_RDWR(tool_link_fops,
> >>> +                     tool_link_read,
> >>> +                     tool_link_write);
> >>>
> >>>   static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
> >>>                              size_t size, loff_t *offp)
> >>> @@ -708,6 +749,9 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
> >>>          debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
> >>>                              tc, &tool_peer_spad_fops);
> >>>
> >>> +       debugfs_create_file("link", S_IRUSR | S_IWUSR, tc->dbgfs,
> >>> +                           tc, &tool_link_fops);
> >>> +
> >>>          mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
> >>>          for (i = 0; i < mw_count; i++) {
> >>>                  char buf[30];
> >>> @@ -741,6 +785,7 @@ static int tool_probe(struct ntb_client *self, struct
> >>> ntb_dev *ntb)
> >>>          }
> >>>
> >>>          tc->ntb = ntb;
> >>> +       init_waitqueue_head(&tc->link_wq);
> >>>          INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
> >>>          INIT_WORK(&tc->link_cleanup, tool_link_cleanup);
> 
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb"
> group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-
> ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-
> ntb/CAJ80sasHLMN3FZnFOKgfU6d7KBmr4zt2H5ej58EapYDBaoqZag%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 6/8] ntb_tool: Add link status file to debugfs

[Logan Gunthorpe]

On 14/06/16 09:45 AM, Allen Hubbe wrote:
> 
> Feel free to disregard my suggestion above.  I hope my comment has not cost you too much time.
> 
> The way you have written it already, and used it in the self-test script is much more concise.
> 
>>> + * root@self# echo > $DBG_DIR/link
> 
> Acked-by: Allen.Hubbe@emc.com
> 
> 
> 
> Eventually, I think it would be useful to let ntb_tool enable and disable the link.  In that case, it might also be useful in a test script to wait for link down, not just link up.
> 
> What about this:
> 
> # Wait for the link to be up or down
> root@self# echo 1 > $DBG_DIR/link
> root@self# echo 0 > $DBG_DIR/link
> 
> It need not be a part of this patch, but eventually:
> 
> # Enable or disable the link
> root@self# echo 1 > $DBG_DIR/link_ctrl
> root@self# echo 0 > $DBG_DIR/link_ctrl
> 
> # Reading the link_ctrl file can also give the link status
> root@self# cat $DBG_DIR/link_ctrl
> 
> Finally, I wonder if the file called "link" in this patch should be called "link_wait" or similar, so its purpose is obviously not for enabling and disabling the link.
> 

Actually I've already implemented something similar to your original
suggestion. I'll be submitting a v2 of this set shortly.

Logan

RE: [PATCH 6/8] ntb_tool: Add link status file to debugfs

[Allen Hubbe]

From: Logan Gunthorpe
> On 14/06/16 09:45 AM, Allen Hubbe wrote:
> >
> > Feel free to disregard my suggestion above.  I hope my comment has not cost you too much
> time.
> >
> > The way you have written it already, and used it in the self-test script is much more
> concise.
> >
> >>> + * root@self# echo > $DBG_DIR/link
> >
> > Acked-by: Allen.Hubbe@emc.com
> >
> >
> >
> > Eventually, I think it would be useful to let ntb_tool enable and disable the link.  In
> that case, it might also be useful in a test script to wait for link down, not just link
> up.
> >
> > What about this:
> >
> > # Wait for the link to be up or down
> > root@self# echo 1 > $DBG_DIR/link
> > root@self# echo 0 > $DBG_DIR/link
> >
> > It need not be a part of this patch, but eventually:
> >
> > # Enable or disable the link
> > root@self# echo 1 > $DBG_DIR/link_ctrl
> > root@self# echo 0 > $DBG_DIR/link_ctrl
> >
> > # Reading the link_ctrl file can also give the link status
> > root@self# cat $DBG_DIR/link_ctrl
> >
> > Finally, I wonder if the file called "link" in this patch should be called "link_wait"
> or similar, so its purpose is obviously not for enabling and disabling the link.
> >
> 
> Actually I've already implemented something similar to your original
> suggestion. I'll be submitting a v2 of this set shortly.

Ok.  Thanks.  I'll accept the blame if anyone doesn't like it.

Re: [PATCH 5/8] ntb_tool: BUG: Ensure the buffer size is large enough to return all spads

[Allen Hubbe]

On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> On hardware with 32 scratchpad registers the spad field in ntb tool
> could chop off the end. The maximum buffer size is increased from
> 256 to 15 times the number or scratchpads.
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

It could be marginally better if there was an explanation to accompany
the magic number 15, but it's not a big deal.  One might guess it has
something to do with the expected length of the formatted string.

Acked-by: Allen Hubbe <Allen.Hubbe@emc.com>

> ---
>  drivers/ntb/test/ntb_tool.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index 4c01057..954e1d5 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -368,7 +368,9 @@ static ssize_t tool_spadfn_read(struct tool_ctx *tc, char __user *ubuf,
>         if (!spad_read_fn)
>                 return -EINVAL;
>
> -       buf_size = min_t(size_t, size, 0x100);
> +       spad_count = ntb_spad_count(tc->ntb);
> +
> +       buf_size = min_t(size_t, size, spad_count * 15);
>
>         buf = kmalloc(buf_size, GFP_KERNEL);
>         if (!buf)
> @@ -376,7 +378,6 @@ static ssize_t tool_spadfn_read(struct tool_ctx *tc, char __user *ubuf,
>
>         pos = 0;
>
> -       spad_count = ntb_spad_count(tc->ntb);
>         for (i = 0; i < spad_count; ++i) {
>                 pos += scnprintf(buf + pos, buf_size - pos, "%d\t%#x\n",
>                                  i, spad_read_fn(tc->ntb, i));
> --
> 2.1.4
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/d9488f2c946644c2b1258a78929d3543747283ec.1465598632.git.logang%40deltatee.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 5/8] ntb_tool: BUG: Ensure the buffer size is large enough to return all spads

[Logan Gunthorpe]

Ok, I'll add a comment.

Logan

On 10/06/16 08:35 PM, Allen Hubbe wrote:
> On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>> On hardware with 32 scratchpad registers the spad field in ntb tool
>> could chop off the end. The maximum buffer size is increased from
>> 256 to 15 times the number or scratchpads.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>
> It could be marginally better if there was an explanation to accompany
> the magic number 15, but it's not a big deal.  One might guess it has
> something to do with the expected length of the formatted string.
>
> Acked-by: Allen Hubbe <Allen.Hubbe@emc.com>
>
>> ---
>>   drivers/ntb/test/ntb_tool.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
>> index 4c01057..954e1d5 100644
>> --- a/drivers/ntb/test/ntb_tool.c
>> +++ b/drivers/ntb/test/ntb_tool.c
>> @@ -368,7 +368,9 @@ static ssize_t tool_spadfn_read(struct tool_ctx *tc, char __user *ubuf,
>>          if (!spad_read_fn)
>>                  return -EINVAL;
>>
>> -       buf_size = min_t(size_t, size, 0x100);
>> +       spad_count = ntb_spad_count(tc->ntb);
>> +
>> +       buf_size = min_t(size_t, size, spad_count * 15);
>>
>>          buf = kmalloc(buf_size, GFP_KERNEL);
>>          if (!buf)
>> @@ -376,7 +378,6 @@ static ssize_t tool_spadfn_read(struct tool_ctx *tc, char __user *ubuf,
>>
>>          pos = 0;
>>
>> -       spad_count = ntb_spad_count(tc->ntb);
>>          for (i = 0; i < spad_count; ++i) {
>>                  pos += scnprintf(buf + pos, buf_size - pos, "%d\t%#x\n",
>>                                   i, spad_read_fn(tc->ntb, i));
>> --
>> 2.1.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
>> To post to this group, send email to linux-ntb@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/d9488f2c946644c2b1258a78929d3543747283ec.1465598632.git.logang%40deltatee.com.
>> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 7/8] ntb_pingpong: Add a debugfs file to get the ping count

[Allen Hubbe]

On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> This commit adds a debugfs 'count' file to ntb_pingpong. This is so
> testing with ntb_pingpong can be automated beyond just checking the
> logs for pong messages.
>
> The count file returns a number which increments every pong. The
> counter can be cleared by writing a zero.
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/ntb/test/ntb_pingpong.c | 68 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ntb/test/ntb_pingpong.c b/drivers/ntb/test/ntb_pingpong.c
> index fe16005..34bbf5a 100644
> --- a/drivers/ntb/test/ntb_pingpong.c
> +++ b/drivers/ntb/test/ntb_pingpong.c
> @@ -61,6 +61,7 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/debugfs.h>
>
>  #include <linux/ntb.h>
>
> @@ -96,8 +97,13 @@ struct pp_ctx {
>         spinlock_t                      db_lock;
>         struct timer_list               db_timer;
>         unsigned long                   db_delay;
> +       struct dentry                   *debugfs_node_dir;
> +       struct dentry                   *debugfs_count;
> +       atomic_t                        count;
>  };
>
> +static struct dentry *pp_debugfs_dir;
> +
>  static void pp_ping(unsigned long ctx)
>  {
>         struct pp_ctx *pp = (void *)ctx;
> @@ -171,10 +177,38 @@ static void pp_db_event(void *ctx, int vec)
>                 dev_dbg(&pp->ntb->dev,
>                         "Pong vec %d bits %#llx\n",
>                         vec, db_bits);
> +               atomic_inc(&pp->count);
>         }
>         spin_unlock_irqrestore(&pp->db_lock, irqflags);
>  }
>
> +static int pp_debugfs_setup(struct pp_ctx *pp)
> +{
> +       struct pci_dev *pdev = pp->ntb->pdev;
> +
> +       if (!debugfs_initialized())
> +               return -ENODEV;
> +
> +       if (!pp_debugfs_dir) {
> +               pp_debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);

The pp_debugfs_dir is already initialized by the module init function.
If it doesn't exist here, I think we should just return instead of
trying again.  It's also worth noting, though it is probably no harm,
the code here does not check debugfs_initialized().

> +               if (!pp_debugfs_dir)
> +                       return -ENODEV;
> +       }
> +
> +       pp->debugfs_node_dir = debugfs_create_dir(pci_name(pdev),
> +                                                 pp_debugfs_dir);
> +       if (!pp->debugfs_node_dir)
> +               return -ENODEV;
> +
> +       pp->debugfs_count = debugfs_create_atomic_t("count", S_IRUSR | S_IWUSR,
> +                                                   pp->debugfs_node_dir,
> +                                                   &pp->count);
> +       if (!pp->debugfs_count)
> +               return -ENODEV;
> +
> +       return 0;
> +}
> +
>  static const struct ntb_ctx_ops pp_ops = {
>         .link_event = pp_link_event,
>         .db_event = pp_db_event,
> @@ -210,6 +244,7 @@ static int pp_probe(struct ntb_client *client,
>
>         pp->ntb = ntb;
>         pp->db_bits = 0;
> +       atomic_set(&pp->count, 0);
>         spin_lock_init(&pp->db_lock);
>         setup_timer(&pp->db_timer, pp_ping, (unsigned long)pp);
>         pp->db_delay = msecs_to_jiffies(delay_ms);
> @@ -218,6 +253,10 @@ static int pp_probe(struct ntb_client *client,
>         if (rc)
>                 goto err_ctx;
>
> +       rc = pp_debugfs_setup(pp);
> +       if (rc)
> +               goto err_ctx;
> +
>         ntb_link_enable(ntb, NTB_SPEED_AUTO, NTB_WIDTH_AUTO);
>         ntb_link_event(ntb);
>
> @@ -234,6 +273,8 @@ static void pp_remove(struct ntb_client *client,
>  {
>         struct pp_ctx *pp = ntb->ctx;
>
> +       debugfs_remove_recursive(pp->debugfs_node_dir);
> +
>         ntb_clear_ctx(ntb);
>         del_timer_sync(&pp->db_timer);
>         ntb_link_disable(ntb);
> @@ -247,4 +288,29 @@ static struct ntb_client pp_client = {
>                 .remove = pp_remove,
>         },
>  };
> -module_ntb_client(pp_client);
> +
> +static int __init tool_init(void)

This should be pp_init() not tool_init().

> +{
> +       int rc;
> +
> +       if (debugfs_initialized())
> +               pp_debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
> +
> +       rc = ntb_register_client(&pp_client);
> +       if (rc)
> +               goto err_client;
> +
> +       return 0;
> +
> +err_client:
> +       debugfs_remove_recursive(pp_debugfs_dir);
> +       return rc;
> +}
> +module_init(tool_init);
> +
> +static void __exit tool_exit(void)
> +{
> +       ntb_unregister_client(&pp_client);
> +       debugfs_remove_recursive(pp_debugfs_dir);
> +}
> +module_exit(tool_exit);

Re: [PATCH 7/8] ntb_pingpong: Add a debugfs file to get the ping count

[Logan Gunthorpe]

> The pp_debugfs_dir is already initialized by the module init function.
> If it doesn't exist here, I think we should just return instead of
> trying again.  It's also worth noting, though it is probably no harm,
> the code here does not check debugfs_initialized().

>> +static int __init tool_init(void)
>
> This should be pp_init() not tool_init().

Yup, this is just sloppy copying on my part. I copied from two different 
places. I'll fix these oversights.

Thanks,

Logan

Re: [PATCH 1/8] ntb_perf: Schedule based on time not on performance

[Jiang, Dave]

On Fri, 2016-06-10 at 16:54 -0600, Logan Gunthorpe wrote:
> When debugging performance problems, if some issue causes the ntb
> hardware to be significantly slower than expected, ntb_perf will
> hang requiring a reboot because it only schedules once every 4GB.
> 
> Instead, schedule based on jiffies so it will not hang the CPU if
> the transfer is slow.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/test/ntb_perf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c
> b/drivers/ntb/test/ntb_perf.c
> index 4368519..5008ccf 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -273,6 +273,7 @@ static int perf_move_data(struct pthr_ctx *pctx,
> char __iomem *dst, char *src,
>  	char __iomem *tmp = dst;
>  	u64 perf, diff_us;
>  	ktime_t kstart, kstop, kdiff;
> +	unsigned long last_sleep = jiffies;
>  
>  	chunks = div64_u64(win_size, buf_size);
>  	total_chunks = div64_u64(total, buf_size);
> @@ -288,8 +289,9 @@ static int perf_move_data(struct pthr_ctx *pctx,
> char __iomem *dst, char *src,
>  		} else
>  			tmp += buf_size;
>  
> -		/* Probably should schedule every 4GB to prevent
> soft hang. */
> -		if (((copied % SZ_4G) == 0) && !use_dma) {
> +		/* Probably should schedule every 5s to prevent soft
> hang. */
> +		if (unlikely((jiffies - last_sleep) > 5 * HZ)) {
> +			last_sleep = jiffies;
>  			set_current_state(TASK_INTERRUPTIBLE);
>  			schedule_timeout(1);
>  		}
> -- 
> 2.1.4
> 

Re: [PATCH 2/8] ntb_perf: Improve thread handling to increase robustness

[Jiang, Dave]

On Fri, 2016-06-10 at 16:54 -0600, Logan Gunthorpe wrote:
> This commit accomplishes a few things:
> 
> 1) Properly prevent multiple sets of threads from running at once
> using
> a mutex. Lots of race issues existed with the thread_cleanup.
> 
> 2) The mutex allows us to ensure that threads are finished before
> tearing down the device or module.
> 
> 3) Don't use kthread_stop when the threads can exit by themselves, as
> this is counter-indicated by the kthread_create documentation.
> Threads
> now wait for kthread_stop to occur.
> 
> 4) Writing to the run file now blocks until the threads are complete.
> The test can then be safely interrupted by a SIGINT.
> 
> Also, while I was at it:
> 
> 5) debugfs_run_write shouldn't return 0 in the early check cases as
> this
> could cause debugfs_run_write to loop undesirably.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/test/ntb_perf.c | 124 +++++++++++++++++++++++++++-------
> ----------
>  1 file changed, 76 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c
> b/drivers/ntb/test/ntb_perf.c
> index 5008ccf..db4dc61 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -58,6 +58,7 @@
>  #include <linux/delay.h>
>  #include <linux/sizes.h>
>  #include <linux/ntb.h>
> +#include <linux/mutex.h>
>  
>  #define DRIVER_NAME		"ntb_perf"
>  #define DRIVER_DESCRIPTION	"PCIe NTB Performance Measurement
> Tool"
> @@ -121,6 +122,7 @@ struct pthr_ctx {
>  	int			dma_prep_err;
>  	int			src_idx;
>  	void			*srcs[MAX_SRCS];
> +	wait_queue_head_t       *wq;
>  };
>  
>  struct perf_ctx {
> @@ -134,9 +136,11 @@ struct perf_ctx {
>  	struct dentry		*debugfs_run;
>  	struct dentry		*debugfs_threads;
>  	u8			perf_threads;
> -	bool			run;
> +	/* mutex ensures only one set of threads run at once */
> +	struct mutex		run_mutex;
>  	struct pthr_ctx		pthr_ctx[MAX_THREADS];
>  	atomic_t		tsync;
> +	atomic_t                tdone;
>  };
>  
>  enum {
> @@ -295,12 +299,18 @@ static int perf_move_data(struct pthr_ctx
> *pctx, char __iomem *dst, char *src,
>  			set_current_state(TASK_INTERRUPTIBLE);
>  			schedule_timeout(1);
>  		}
> +
> +		if (unlikely(kthread_should_stop()))
> +			break;
>  	}
>  
>  	if (use_dma) {
>  		pr_info("%s: All DMA descriptors submitted\n",
> current->comm);
> -		while (atomic_read(&pctx->dma_sync) != 0)
> +		while (atomic_read(&pctx->dma_sync) != 0) {
> +			if (kthread_should_stop())
> +				break;
>  			msleep(20);
> +		}
>  	}
>  
>  	kstop = ktime_get();
> @@ -393,7 +403,10 @@ static int ntb_perf_thread(void *data)
>  		pctx->srcs[i] = NULL;
>  	}
>  
> -	return 0;
> +	atomic_inc(&perf->tdone);
> +	wake_up(pctx->wq);
> +	rc = 0;
> +	goto done;
>  
>  err:
>  	for (i = 0; i < MAX_SRCS; i++) {
> @@ -406,6 +419,16 @@ err:
>  		pctx->dma_chan = NULL;
>  	}
>  
> +done:
> +	/* Wait until we are told to stop */
> +	for (;;) {
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		if (kthread_should_stop())
> +			break;
> +		schedule();
> +	}
> +	__set_current_state(TASK_RUNNING);
> +
>  	return rc;
>  }
>  
> @@ -553,6 +576,7 @@ static ssize_t debugfs_run_read(struct file
> *filp, char __user *ubuf,
>  	struct perf_ctx *perf = filp->private_data;
>  	char *buf;
>  	ssize_t ret, out_offset;
> +	int running;
>  
>  	if (!perf)
>  		return 0;
> @@ -560,7 +584,9 @@ static ssize_t debugfs_run_read(struct file
> *filp, char __user *ubuf,
>  	buf = kmalloc(64, GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
> -	out_offset = snprintf(buf, 64, "%d\n", perf->run);
> +
> +	running = mutex_is_locked(&perf->run_mutex);
> +	out_offset = snprintf(buf, 64, "%d\n", running);
>  	ret = simple_read_from_buffer(ubuf, count, offp, buf,
> out_offset);
>  	kfree(buf);
>  
> @@ -572,7 +598,6 @@ static void threads_cleanup(struct perf_ctx
> *perf)
>  	struct pthr_ctx *pctx;
>  	int i;
>  
> -	perf->run = false;
>  	for (i = 0; i < MAX_THREADS; i++) {
>  		pctx = &perf->pthr_ctx[i];
>  		if (pctx->thread) {
> @@ -587,65 +612,66 @@ static ssize_t debugfs_run_write(struct file
> *filp, const char __user *ubuf,
>  {
>  	struct perf_ctx *perf = filp->private_data;
>  	int node, i;
> +	DECLARE_WAIT_QUEUE_HEAD(wq);
>  
>  	if (!perf->link_is_up)
> -		return 0;
> +		return -ENOLINK;
>  
>  	if (perf->perf_threads == 0)
> -		return 0;
> +		return -EINVAL;
>  
> -	if (atomic_read(&perf->tsync) == 0)
> -		perf->run = false;
> +	if (!mutex_trylock(&perf->run_mutex))
> +		return -EBUSY;
>  
> -	if (perf->run)
> -		threads_cleanup(perf);
> -	else {
> -		perf->run = true;
> +	if (perf->perf_threads > MAX_THREADS) {
> +		perf->perf_threads = MAX_THREADS;
> +		pr_info("Reset total threads to: %u\n",
> MAX_THREADS);
> +	}
>  
> -		if (perf->perf_threads > MAX_THREADS) {
> -			perf->perf_threads = MAX_THREADS;
> -			pr_info("Reset total threads to: %u\n",
> MAX_THREADS);
> -		}
> +	/* no greater than 1M */
> +	if (seg_order > MAX_SEG_ORDER) {
> +		seg_order = MAX_SEG_ORDER;
> +		pr_info("Fix seg_order to %u\n", seg_order);
> +	}
>  
> -		/* no greater than 1M */
> -		if (seg_order > MAX_SEG_ORDER) {
> -			seg_order = MAX_SEG_ORDER;
> -			pr_info("Fix seg_order to %u\n", seg_order);
> -		}
> +	if (run_order < seg_order) {
> +		run_order = seg_order;
> +		pr_info("Fix run_order to %u\n", run_order);
> +	}
>  
> -		if (run_order < seg_order) {
> -			run_order = seg_order;
> -			pr_info("Fix run_order to %u\n", run_order);
> -		}
> +	node = dev_to_node(&perf->ntb->pdev->dev);
> +	atomic_set(&perf->tdone, 0);
>  
> -		node = dev_to_node(&perf->ntb->pdev->dev);
> -		/* launch kernel thread */
> -		for (i = 0; i < perf->perf_threads; i++) {
> -			struct pthr_ctx *pctx;
> -
> -			pctx = &perf->pthr_ctx[i];
> -			atomic_set(&pctx->dma_sync, 0);
> -			pctx->perf = perf;
> -			pctx->thread =
> -				kthread_create_on_node(ntb_perf_thre
> ad,
> -						       (void *)pctx,
> -						       node,
> "ntb_perf %d", i);
> -			if (IS_ERR(pctx->thread)) {
> -				pctx->thread = NULL;
> -				goto err;
> -			} else
> -				wake_up_process(pctx->thread);
> -
> -			if (perf->run == false)
> -				return -ENXIO;
> -		}
> +	/* launch kernel thread */
> +	for (i = 0; i < perf->perf_threads; i++) {
> +		struct pthr_ctx *pctx;
>  
> +		pctx = &perf->pthr_ctx[i];
> +		atomic_set(&pctx->dma_sync, 0);
> +		pctx->perf = perf;
> +		pctx->wq = &wq;
> +		pctx->thread =
> +			kthread_create_on_node(ntb_perf_thread,
> +					       (void *)pctx,
> +					       node, "ntb_perf %d",
> i);
> +		if (IS_ERR(pctx->thread)) {
> +			pctx->thread = NULL;
> +			goto err;
> +		} else {
> +			wake_up_process(pctx->thread);
> +		}
>  	}
>  
> +	wait_event_interruptible(wq,
> +		atomic_read(&perf->tdone) == perf->perf_threads);
> +
> +	threads_cleanup(perf);
> +	mutex_unlock(&perf->run_mutex);
>  	return count;
>  
>  err:
>  	threads_cleanup(perf);
> +	mutex_unlock(&perf->run_mutex);
>  	return -ENXIO;
>  }
>  
> @@ -713,7 +739,7 @@ static int perf_probe(struct ntb_client *client,
> struct ntb_dev *ntb)
>  	perf->ntb = ntb;
>  	perf->perf_threads = 1;
>  	atomic_set(&perf->tsync, 0);
> -	perf->run = false;
> +	mutex_init(&perf->run_mutex);
>  	spin_lock_init(&perf->db_lock);
>  	perf_setup_mw(ntb, perf);
>  	INIT_DELAYED_WORK(&perf->link_work, perf_link_work);
> @@ -748,6 +774,8 @@ static void perf_remove(struct ntb_client
> *client, struct ntb_dev *ntb)
>  
>  	dev_dbg(&perf->ntb->dev, "%s called\n", __func__);
>  
> +	mutex_lock(&perf->run_mutex);
> +
>  	cancel_delayed_work_sync(&perf->link_work);
>  	cancel_work_sync(&perf->link_cleanup);
>  
> -- 
> 2.1.4
> 

Re: [PATCH 3/8] ntb_perf: Return results by reading the run file

[Jiang, Dave]

On Fri, 2016-06-10 at 16:54 -0600, Logan Gunthorpe wrote:
> Instead of having to watch logs, allow the results to be retrieved
> by reading back the run file. This file will return "running" when
> the test is running and nothing if no tests have been run yet.
> It returns 1 line per thread, and will display an error message if
> the
> corresponding thread returns an error.
> 
> With the above change, the pr_info calls that returned the results
> are
> then changed to pr_debug calls.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/test/ntb_perf.c | 67
> +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 55 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c
> b/drivers/ntb/test/ntb_perf.c
> index db4dc61..05a8705 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -123,6 +123,9 @@ struct pthr_ctx {
>  	int			src_idx;
>  	void			*srcs[MAX_SRCS];
>  	wait_queue_head_t       *wq;
> +	int			status;
> +	u64			copied;
> +	u64			diff_us;
>  };
>  
>  struct perf_ctx {
> @@ -305,7 +308,7 @@ static int perf_move_data(struct pthr_ctx *pctx,
> char __iomem *dst, char *src,
>  	}
>  
>  	if (use_dma) {
> -		pr_info("%s: All DMA descriptors submitted\n",
> current->comm);
> +		pr_debug("%s: All DMA descriptors submitted\n",
> current->comm);
>  		while (atomic_read(&pctx->dma_sync) != 0) {
>  			if (kthread_should_stop())
>  				break;
> @@ -317,13 +320,16 @@ static int perf_move_data(struct pthr_ctx
> *pctx, char __iomem *dst, char *src,
>  	kdiff = ktime_sub(kstop, kstart);
>  	diff_us = ktime_to_us(kdiff);
>  
> -	pr_info("%s: copied %llu bytes\n", current->comm, copied);
> +	pr_debug("%s: copied %llu bytes\n", current->comm, copied);
>  
> -	pr_info("%s: lasted %llu usecs\n", current->comm, diff_us);
> +	pr_debug("%s: lasted %llu usecs\n", current->comm, diff_us);
>  
>  	perf = div64_u64(copied, diff_us);
>  
> -	pr_info("%s: MBytes/s: %llu\n", current->comm, perf);
> +	pr_debug("%s: MBytes/s: %llu\n", current->comm, perf);
> +
> +	pctx->copied = copied;
> +	pctx->diff_us = diff_us;
>  
>  	return 0;
>  }
> @@ -345,7 +351,7 @@ static int ntb_perf_thread(void *data)
>  	int rc, node, i;
>  	struct dma_chan *dma_chan = NULL;
>  
> -	pr_info("kthread %s starting...\n", current->comm);
> +	pr_debug("kthread %s starting...\n", current->comm);
>  
>  	node = dev_to_node(&pdev->dev);
>  
> @@ -575,19 +581,44 @@ static ssize_t debugfs_run_read(struct file
> *filp, char __user *ubuf,
>  {
>  	struct perf_ctx *perf = filp->private_data;
>  	char *buf;
> -	ssize_t ret, out_offset;
> -	int running;
> +	ssize_t ret, out_off = 0;
> +	struct pthr_ctx *pctx;
> +	int i;
> +	u64 rate;
>  
>  	if (!perf)
>  		return 0;
>  
> -	buf = kmalloc(64, GFP_KERNEL);
> +	buf = kmalloc(1024, GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
>  
> -	running = mutex_is_locked(&perf->run_mutex);
> -	out_offset = snprintf(buf, 64, "%d\n", running);
> -	ret = simple_read_from_buffer(ubuf, count, offp, buf,
> out_offset);
> +	if (mutex_is_locked(&perf->run_mutex)) {
> +		out_off = snprintf(buf, 64, "running\n");
> +		goto read_from_buf;
> +	}
> +
> +	for (i = 0; i < MAX_THREADS; i++) {
> +		pctx = &perf->pthr_ctx[i];
> +
> +		if (pctx->status == -ENODATA)
> +			break;
> +
> +		if (pctx->status) {
> +			out_off += snprintf(buf + out_off, 1024 -
> out_off,
> +					    "%d: error %d\n", i,
> +					    pctx->status);
> +			continue;
> +		}
> +
> +		rate = div64_u64(pctx->copied, pctx->diff_us);
> +		out_off += snprintf(buf + out_off, 1024 - out_off,
> +			"%d: copied %llu bytes in %llu usecs, %llu
> MBytes/s\n",
> +			i, pctx->copied, pctx->diff_us, rate);
> +	}
> +
> +read_from_buf:
> +	ret = simple_read_from_buffer(ubuf, count, offp, buf,
> out_off);
>  	kfree(buf);
>  
>  	return ret;
> @@ -601,12 +632,20 @@ static void threads_cleanup(struct perf_ctx
> *perf)
>  	for (i = 0; i < MAX_THREADS; i++) {
>  		pctx = &perf->pthr_ctx[i];
>  		if (pctx->thread) {
> -			kthread_stop(pctx->thread);
> +			pctx->status = kthread_stop(pctx->thread);
>  			pctx->thread = NULL;
>  		}
>  	}
>  }
>  
> +static void perf_clear_thread_status(struct perf_ctx *perf)
> +{
> +	int i;
> +
> +	for (i = 0; i < MAX_THREADS; i++)
> +		perf->pthr_ctx[i].status = -ENODATA;
> +}
> +
>  static ssize_t debugfs_run_write(struct file *filp, const char
> __user *ubuf,
>  				 size_t count, loff_t *offp)
>  {
> @@ -623,6 +662,8 @@ static ssize_t debugfs_run_write(struct file
> *filp, const char __user *ubuf,
>  	if (!mutex_trylock(&perf->run_mutex))
>  		return -EBUSY;
>  
> +	perf_clear_thread_status(perf);
> +
>  	if (perf->perf_threads > MAX_THREADS) {
>  		perf->perf_threads = MAX_THREADS;
>  		pr_info("Reset total threads to: %u\n",
> MAX_THREADS);
> @@ -757,6 +798,8 @@ static int perf_probe(struct ntb_client *client,
> struct ntb_dev *ntb)
>  	if (rc)
>  		goto err_ctx;
>  
> +	perf_clear_thread_status(perf);
> +
>  	return 0;
>  
>  err_ctx:
> -- 
> 2.1.4
> 

Re: [PATCH 4/8] ntb_perf: Wait for link before running test

[Jiang, Dave]

On Fri, 2016-06-10 at 16:54 -0600, Logan Gunthorpe wrote:
> Instead of returning immediately with an error when the link is
> down, wait for the link to come up (or the user sends a SIGINT).
> 
> This is to make scripting ntb_perf easier.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/test/ntb_perf.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c
> b/drivers/ntb/test/ntb_perf.c
> index 05a8705..f0784e5 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -135,6 +135,7 @@ struct perf_ctx {
>  	bool			link_is_up;
>  	struct work_struct	link_cleanup;
>  	struct delayed_work	link_work;
> +	wait_queue_head_t	link_wq;
>  	struct dentry		*debugfs_node_dir;
>  	struct dentry		*debugfs_run;
>  	struct dentry		*debugfs_threads;
> @@ -533,6 +534,7 @@ static void perf_link_work(struct work_struct
> *work)
>  		goto out1;
>  
>  	perf->link_is_up = true;
> +	wake_up(&perf->link_wq);
>  
>  	return;
>  
> @@ -653,7 +655,7 @@ static ssize_t debugfs_run_write(struct file
> *filp, const char __user *ubuf,
>  	int node, i;
>  	DECLARE_WAIT_QUEUE_HEAD(wq);
>  
> -	if (!perf->link_is_up)
> +	if (wait_event_interruptible(perf->link_wq, perf-
> >link_is_up))
>  		return -ENOLINK;
>  
>  	if (perf->perf_threads == 0)
> @@ -783,6 +785,7 @@ static int perf_probe(struct ntb_client *client,
> struct ntb_dev *ntb)
>  	mutex_init(&perf->run_mutex);
>  	spin_lock_init(&perf->db_lock);
>  	perf_setup_mw(ntb, perf);
> +	init_waitqueue_head(&perf->link_wq);
>  	INIT_DELAYED_WORK(&perf->link_work, perf_link_work);
>  	INIT_WORK(&perf->link_cleanup, perf_link_cleanup);
>  
> -- 
> 2.1.4
> 

Re: [PATCH 8/8] ntb_test: Add a selftest script for the NTB subsystem

[Jon Mason]

On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> This script automates testing doorbells, scratchpads and memory windows
> for an NTB device. It can be run locally, with the NTB looped
> back to the same host or use SSH to remotely control the second host.
>
> In the single host case, the script just needs to be passed two
> arguments: a PCI ID for each side of the link. In the two host case
> the -r option must be used to specify the remote hostname (which must
> be SSH accessible and should probably have ssh-keys exchanged).

I appreciate the work that you are putting in here, but test shell
scripts are not accepted into the kernel source.

I think a better place for this to be shared would be on the github
account wiki, https://github.com/jonmason/ntb/wiki
In fact, I'd really like for someone to add some pages there on using
the ntb tools and testing.  If you are willing, I'd be most
appreciative.

Thanks,
Jon

>
> A sample run looks like this:
>
> $ sudo ./ntb_test.sh 0000:03:00.1 0000:83:00.1 -p 29
> Starting ntb_tool tests...
> Running db tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running db tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
> Running spad tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running spad tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
> Running mw0 tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running mw0 tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
> Running mw1 tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running mw1 tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
>
> Starting ntb_pingpong tests...
> Running ping pong tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
>
> Starting ntb_perf tests...
> Running local perf test without DMA
>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>   Passed
> Running remote perf test without DMA
>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>   Passed
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  MAINTAINERS                             |   1 +
>  tools/testing/selftests/ntb/ntb_test.sh | 386 ++++++++++++++++++++++++++++++++
>  2 files changed, 387 insertions(+)
>  create mode 100755 tools/testing/selftests/ntb/ntb_test.sh
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9c567a4..f178e7e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7846,6 +7846,7 @@ F:        drivers/ntb/
>  F:     drivers/net/ntb_netdev.c
>  F:     include/linux/ntb.h
>  F:     include/linux/ntb_transport.h
> +F:     tools/testing/selftests/ntb/
>
>  NTB INTEL DRIVER
>  M:     Jon Mason <jdmason@kudzu.us>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
> new file mode 100755
> index 0000000..e4a89e9
> --- /dev/null
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -0,0 +1,386 @@
> +#!/bin/bash
> +# Copyright (c) 2016 Microsemi. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# Author: Logan Gunthorpe <logang@deltatee.com>
> +
> +REMOTE_HOST=
> +LIST_DEVS=FALSE
> +
> +DEBUGFS=${DEBUGFS-/sys/kernel/debug}
> +
> +PERF_RUN_ORDER=32
> +MAX_MW_SIZE=0
> +RUN_DMA_TESTS=
> +DONT_CLEANUP=
> +
> +function show_help()
> +{
> +       echo "Usage: $0 [OPTIONS] LOCAL_DEV REMOTE_DEV"
> +       echo "Run tests on a pair of NTB endpoints."
> +       echo
> +       echo "If the NTB device loops back to the same host then,"
> +       echo "just specifying the two PCI ids on the command line is"
> +       echo "sufficient. Otherwise, if the NTB link spans two hosts"
> +       echo "use the -r option to specify the hostname for the remote"
> +       echo "device. SSH will then be used to test the remote side."
> +       echo "An SSH key between the root users of the host would then"
> +       echo "be highly recommended."
> +       echo
> +       echo "Options:"
> +       echo "  -C              don't cleanup ntb modules on exit"
> +       echo "  -d              run dma tests"
> +       echo "  -h              show this help message"
> +       echo "  -l              list available local and remote PCI ids"
> +       echo "  -r REMOTE_HOST  specify the remote's hostname to connect"
> +        echo "                  to for the test (using ssh)"
> +       echo "  -p NUM          ntb_perf run order (default: $PERF_RUN_ORDER)"
> +       echo "  -w max_mw_size  maxmium memory window size"
> +       echo
> +}
> +
> +function parse_args()
> +{
> +       OPTIND=0
> +       while getopts "Cdhlr:p:w:" opt; do
> +               case "$opt" in
> +               C)  DONT_CLEANUP=1 ;;
> +               d)  RUN_DMA_TESTS=1 ;;
> +               h)  show_help; exit 0 ;;
> +               l)  LIST_DEVS=TRUE ;;
> +               r)  REMOTE_HOST=${OPTARG} ;;
> +               p)  PERF_RUN_ORDER=${OPTARG} ;;
> +               w)  MAX_MW_SIZE=${OPTARG} ;;
> +               \?)
> +                   echo "Invalid option: -$OPTARG" >&2
> +                   exit 1
> +                   ;;
> +               esac
> +       done
> +}
> +
> +parse_args "$@"
> +shift $((OPTIND-1))
> +LOCAL_DEV=$1
> +shift
> +parse_args "$@"
> +shift $((OPTIND-1))
> +REMOTE_DEV=$1
> +shift
> +parse_args "$@"
> +
> +set -e
> +
> +function _modprobe()
> +{
> +        modprobe "$@"
> +}
> +
> +function split_remote()
> +{
> +       VPATH=$1
> +       REMOTE=
> +
> +       if [[ "$VPATH" == *":/"* ]]; then
> +               REMOTE=${VPATH%%:*}
> +               VPATH=${VPATH#*:}
> +       fi
> +}
> +
> +function read_file()
> +{
> +       split_remote $1
> +       if [[ "$REMOTE" != "" ]]; then
> +               ssh "$REMOTE" cat "$VPATH"
> +       else
> +               cat "$VPATH"
> +       fi
> +}
> +
> +function write_file()
> +{
> +       split_remote $2
> +       VALUE=$1
> +
> +       if [[ "$REMOTE" != "" ]]; then
> +               ssh "$REMOTE" "echo \"$VALUE\" > \"$VPATH\""
> +       else
> +               echo "$VALUE" > "$VPATH"
> +       fi
> +}
> +
> +function doorbell_test()
> +{
> +       LOC=$1
> +       REM=$2
> +       EXP=0
> +
> +       echo "Running db tests on: $(basename $LOC) / $(basename $REM)"
> +
> +       write_file "c 0xFFFFFFFF" "$REM/db"
> +
> +       for ((i=1; i <= 8; i++)); do
> +               let DB=$(read_file "$REM/db") || true
> +               if [[ "$DB" != "$EXP" ]]; then
> +                       echo "Doorbell doesn't match expected value $EXP " \
> +                            "in $REM/db" >&2
> +                       exit -1
> +               fi
> +
> +               let "MASK=1 << ($i-1)" || true
> +               let "EXP=$EXP | $MASK" || true
> +               write_file "s $MASK" "$LOC/peer_db"
> +       done
> +
> +       echo "  Passed"
> +}
> +
> +function read_spad()
> +{
> +       VPATH=$1
> +       IDX=$2
> +
> +       ROW=($(read_file "$VPATH" | grep -e "^$IDX"))
> +       let VAL=${ROW[1]} || true
> +       echo $VAL
> +}
> +
> +function scratchpad_test()
> +{
> +       LOC=$1
> +       REM=$2
> +       CNT=$(read_file "$LOC/spad" | wc -l)
> +
> +       echo "Running spad tests on: $(basename $LOC) / $(basename $REM)"
> +
> +       for ((i = 0; i < $CNT; i++)); do
> +               VAL=$RANDOM
> +               write_file "$i $VAL" "$LOC/peer_spad"
> +               RVAL=$(read_spad "$REM/spad" $i)
> +
> +               if [[ "$VAL" != "$RVAL" ]]; then
> +                       echo "Scratchpad doesn't match expected value $VAL " \
> +                            "in $REM/spad, got $RVAL" >&2
> +                       exit -1
> +               fi
> +
> +       done
> +
> +       echo "  Passed"
> +}
> +
> +function write_mw()
> +{
> +       split_remote $2
> +
> +       if [[ "$REMOTE" != "" ]]; then
> +               ssh "$REMOTE" \
> +                       dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
> +       else
> +               dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
> +       fi
> +}
> +
> +function mw_test()
> +{
> +       IDX=$1
> +       LOC=$2
> +       REM=$3
> +
> +       echo "Running $IDX tests on: $(basename $LOC) / $(basename $REM)"
> +
> +       write_mw "$LOC/$IDX"
> +
> +       split_remote "$LOC/$IDX"
> +       if [[ "$REMOTE" == "" ]]; then
> +               A=$VPATH
> +       else
> +               A=/tmp/ntb_test.$$.A
> +               ssh "$REMOTE" cat "$VPATH" > "$A"
> +       fi
> +
> +       split_remote "$REM/peer_$IDX"
> +       if [[ "$REMOTE" == "" ]]; then
> +               B=$VPATH
> +       else
> +               B=/tmp/ntb_test.$$.B
> +               ssh "$REMOTE" cat "$VPATH" > "$B"
> +       fi
> +
> +       cmp "$A" "$B"
> +       if [[ $? != 0 ]]; then
> +               echo "Memory window $MW did not match!" >&2
> +       fi
> +
> +       if [[ "$A" == "/tmp/*" ]]; then
> +               rm "$A"
> +       fi
> +
> +       if [[ "$B" == "/tmp/*" ]]; then
> +               rm "$B"
> +       fi
> +
> +       echo "  Passed"
> +}
> +
> +function pingpong_test()
> +{
> +       LOC=$1
> +       REM=$2
> +
> +       echo "Running ping pong tests on: $(basename $LOC) / $(basename $REM)"
> +
> +       LOC_START=$(read_file $LOC/count)
> +       REM_START=$(read_file $REM/count)
> +
> +       sleep 7
> +
> +       LOC_END=$(read_file $LOC/count)
> +       REM_END=$(read_file $REM/count)
> +
> +       if [[ $LOC_START == $LOC_END ]] || [[ $REM_START == $REM_END ]]; then
> +               echo "Ping pong counter not incrementing!" >&2
> +               exit 1
> +       fi
> +
> +       echo "  Passed"
> +}
> +
> +function perf_test()
> +{
> +       USE_DMA=$1
> +
> +       if [[ $USE_DMA == "1" ]]; then
> +               WITH="with"
> +       else
> +               WITH="without"
> +       fi
> +
> +       _modprobe ntb_perf run_order=$PERF_RUN_ORDER \
> +               max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA
> +
> +       echo "Running local perf test $WITH DMA"
> +       write_file "" $LOCAL_PERF/run
> +       echo -n "  "
> +       read_file $LOCAL_PERF/run
> +       echo "  Passed"
> +
> +       echo "Running remote perf test $WITH DMA"
> +       write_file "" $REMOTE_PERF/run
> +       echo -n "  "
> +       read_file $LOCAL_PERF/run
> +       echo "  Passed"
> +
> +       _modprobe -r ntb_perf
> +}
> +
> +function ntb_tool_tests()
> +{
> +       LOCAL_TOOL=$DEBUGFS/ntb_tool/$LOCAL_DEV
> +       REMOTE_TOOL=$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV
> +
> +       echo "Starting ntb_tool tests..."
> +
> +       _modprobe ntb_tool
> +
> +       echo > $LOCAL_TOOL/link
> +
> +       doorbell_test $LOCAL_TOOL $REMOTE_TOOL
> +       doorbell_test $REMOTE_TOOL $LOCAL_TOOL
> +       scratchpad_test $LOCAL_TOOL $REMOTE_TOOL
> +       scratchpad_test $REMOTE_TOOL $LOCAL_TOOL
> +
> +       for MW in $(ls $LOCAL_TOOL/mw*); do
> +               MW=$(basename $MW)
> +               mw_test $MW $LOCAL_TOOL $REMOTE_TOOL
> +               mw_test $MW $REMOTE_TOOL $LOCAL_TOOL
> +       done
> +
> +       _modprobe -r ntb_tool
> +}
> +
> +function ntb_pingpong_tests()
> +{
> +       LOCAL_PP=$DEBUGFS/ntb_pingpong/$LOCAL_DEV
> +       REMOTE_PP=$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV
> +
> +       echo "Starting ntb_pingpong tests..."
> +
> +       _modprobe ntb_pingpong
> +
> +       pingpong_test $LOCAL_PP $REMOTE_PP
> +
> +       _modprobe -r ntb_pingpong
> +}
> +
> +function ntb_perf_tests()
> +{
> +       LOCAL_PERF=$DEBUGFS/ntb_perf/$LOCAL_DEV
> +       REMOTE_PERF=$REMOTE_HOST:$DEBUGFS/ntb_perf/$REMOTE_DEV
> +
> +       echo "Starting ntb_perf tests..."
> +
> +       perf_test 0
> +
> +       if [[ $RUN_DMA_TESTS ]]; then
> +               perf_test 1
> +       fi
> +}
> +
> +function cleanup()
> +{
> +       set +e
> +       _modprobe -r ntb_tool 2> /dev/null
> +       _modprobe -r ntb_perf 2> /dev/null
> +       _modprobe -r ntb_pingpong 2> /dev/null
> +       _modprobe -r ntb_transport 2> /dev/null
> +       set -e
> +}
> +
> +cleanup
> +
> +if ! [[ $$DONT_CLEANUP ]]; then
> +       trap cleanup EXIT
> +fi
> +
> +if [ "$(id -u)" != "0" ]; then
> +       echo "This script must be run as root" 1>&2
> +       exit 1
> +fi
> +
> +if [[ "$LIST_DEVS" == TRUE ]]; then
> +       _modprobe ntb_tool
> +       echo "Local Devices:"
> +       ls -1 /sys/kernel/debug/ntb_tool
> +       echo
> +
> +       if [[ "$REMOTE_HOST" != "" ]]; then
> +               echo "Remote Devices:"
> +               ssh $REMOTE_HOST ls -1 /sys/kernel/debug/ntb_tool
> +       fi
> +
> +       _modprobe -r ntb_tool
> +
> +       exit 0
> +fi
> +
> +if [[ "$LOCAL_DEV" == $"" ]] || [[ "$REMOTE_DEV" == $"" ]]; then
> +       show_help
> +       exit 1
> +fi
> +
> +ntb_tool_tests
> +echo
> +ntb_pingpong_tests
> +echo
> +ntb_perf_tests
> +echo
> --
> 2.1.4
>

Re: [PATCH 8/8] ntb_test: Add a selftest script for the NTB subsystem

[Shuah Khan]

On 06/14/2016 08:06 AM, Jon Mason wrote:
> On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>> This script automates testing doorbells, scratchpads and memory windows
>> for an NTB device. It can be run locally, with the NTB looped
>> back to the same host or use SSH to remotely control the second host.
>>
>> In the single host case, the script just needs to be passed two
>> arguments: a PCI ID for each side of the link. In the two host case
>> the -r option must be used to specify the remote hostname (which must
>> be SSH accessible and should probably have ssh-keys exchanged).
> 
> I appreciate the work that you are putting in here, but test shell
> scripts are not accepted into the kernel source.

I don't see any reason for this script to be not part of kernel selftests.
I think it will be a good addition. We probably don't want to include it in
the auto run of the selftest suite.

Jon! I you would like to take this script through your ntb tree, here is
my ack for the script for kselftest part.

Acked-by: Shuah Khan <shuahkh@osg.samsung.com>

thanks,
-- Shuah

> 
> I think a better place for this to be shared would be on the github
> account wiki, https://github.com/jonmason/ntb/wiki
> In fact, I'd really like for someone to add some pages there on using
> the ntb tools and testing.  If you are willing, I'd be most
> appreciative.
> 
> Thanks,
> Jon
> 
>>
>> A sample run looks like this:
>>
>> $ sudo ./ntb_test.sh 0000:03:00.1 0000:83:00.1 -p 29
>> Starting ntb_tool tests...
>> Running db tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running db tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>> Running spad tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running spad tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>> Running mw0 tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running mw0 tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>> Running mw1 tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running mw1 tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>>
>> Starting ntb_pingpong tests...
>> Running ping pong tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>>
>> Starting ntb_perf tests...
>> Running local perf test without DMA
>>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>>   Passed
>> Running remote perf test without DMA
>>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>>   Passed
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> ---
>>  MAINTAINERS                             |   1 +
>>  tools/testing/selftests/ntb/ntb_test.sh | 386 ++++++++++++++++++++++++++++++++
>>  2 files changed, 387 insertions(+)
>>  create mode 100755 tools/testing/selftests/ntb/ntb_test.sh
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 9c567a4..f178e7e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7846,6 +7846,7 @@ F:        drivers/ntb/
>>  F:     drivers/net/ntb_netdev.c
>>  F:     include/linux/ntb.h
>>  F:     include/linux/ntb_transport.h
>> +F:     tools/testing/selftests/ntb/
>>
>>  NTB INTEL DRIVER
>>  M:     Jon Mason <jdmason@kudzu.us>
>> diff --git a/tools/testing/selftests/ntb/ntb_test.sh b/tools/testing/selftests/ntb/ntb_test.sh
>> new file mode 100755
>> index 0000000..e4a89e9
>> --- /dev/null
>> +++ b/tools/testing/selftests/ntb/ntb_test.sh
>> @@ -0,0 +1,386 @@
>> +#!/bin/bash
>> +# Copyright (c) 2016 Microsemi. All Rights Reserved.
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation; either version 2 of
>> +# the License, or (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# Author: Logan Gunthorpe <logang@deltatee.com>
>> +
>> +REMOTE_HOST=
>> +LIST_DEVS=FALSE
>> +
>> +DEBUGFS=${DEBUGFS-/sys/kernel/debug}
>> +
>> +PERF_RUN_ORDER=32
>> +MAX_MW_SIZE=0
>> +RUN_DMA_TESTS=
>> +DONT_CLEANUP=
>> +
>> +function show_help()
>> +{
>> +       echo "Usage: $0 [OPTIONS] LOCAL_DEV REMOTE_DEV"
>> +       echo "Run tests on a pair of NTB endpoints."
>> +       echo
>> +       echo "If the NTB device loops back to the same host then,"
>> +       echo "just specifying the two PCI ids on the command line is"
>> +       echo "sufficient. Otherwise, if the NTB link spans two hosts"
>> +       echo "use the -r option to specify the hostname for the remote"
>> +       echo "device. SSH will then be used to test the remote side."
>> +       echo "An SSH key between the root users of the host would then"
>> +       echo "be highly recommended."
>> +       echo
>> +       echo "Options:"
>> +       echo "  -C              don't cleanup ntb modules on exit"
>> +       echo "  -d              run dma tests"
>> +       echo "  -h              show this help message"
>> +       echo "  -l              list available local and remote PCI ids"
>> +       echo "  -r REMOTE_HOST  specify the remote's hostname to connect"
>> +        echo "                  to for the test (using ssh)"
>> +       echo "  -p NUM          ntb_perf run order (default: $PERF_RUN_ORDER)"
>> +       echo "  -w max_mw_size  maxmium memory window size"
>> +       echo
>> +}
>> +
>> +function parse_args()
>> +{
>> +       OPTIND=0
>> +       while getopts "Cdhlr:p:w:" opt; do
>> +               case "$opt" in
>> +               C)  DONT_CLEANUP=1 ;;
>> +               d)  RUN_DMA_TESTS=1 ;;
>> +               h)  show_help; exit 0 ;;
>> +               l)  LIST_DEVS=TRUE ;;
>> +               r)  REMOTE_HOST=${OPTARG} ;;
>> +               p)  PERF_RUN_ORDER=${OPTARG} ;;
>> +               w)  MAX_MW_SIZE=${OPTARG} ;;
>> +               \?)
>> +                   echo "Invalid option: -$OPTARG" >&2
>> +                   exit 1
>> +                   ;;
>> +               esac
>> +       done
>> +}
>> +
>> +parse_args "$@"
>> +shift $((OPTIND-1))
>> +LOCAL_DEV=$1
>> +shift
>> +parse_args "$@"
>> +shift $((OPTIND-1))
>> +REMOTE_DEV=$1
>> +shift
>> +parse_args "$@"
>> +
>> +set -e
>> +
>> +function _modprobe()
>> +{
>> +        modprobe "$@"
>> +}
>> +
>> +function split_remote()
>> +{
>> +       VPATH=$1
>> +       REMOTE=
>> +
>> +       if [[ "$VPATH" == *":/"* ]]; then
>> +               REMOTE=${VPATH%%:*}
>> +               VPATH=${VPATH#*:}
>> +       fi
>> +}
>> +
>> +function read_file()
>> +{
>> +       split_remote $1
>> +       if [[ "$REMOTE" != "" ]]; then
>> +               ssh "$REMOTE" cat "$VPATH"
>> +       else
>> +               cat "$VPATH"
>> +       fi
>> +}
>> +
>> +function write_file()
>> +{
>> +       split_remote $2
>> +       VALUE=$1
>> +
>> +       if [[ "$REMOTE" != "" ]]; then
>> +               ssh "$REMOTE" "echo \"$VALUE\" > \"$VPATH\""
>> +       else
>> +               echo "$VALUE" > "$VPATH"
>> +       fi
>> +}
>> +
>> +function doorbell_test()
>> +{
>> +       LOC=$1
>> +       REM=$2
>> +       EXP=0
>> +
>> +       echo "Running db tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +       write_file "c 0xFFFFFFFF" "$REM/db"
>> +
>> +       for ((i=1; i <= 8; i++)); do
>> +               let DB=$(read_file "$REM/db") || true
>> +               if [[ "$DB" != "$EXP" ]]; then
>> +                       echo "Doorbell doesn't match expected value $EXP " \
>> +                            "in $REM/db" >&2
>> +                       exit -1
>> +               fi
>> +
>> +               let "MASK=1 << ($i-1)" || true
>> +               let "EXP=$EXP | $MASK" || true
>> +               write_file "s $MASK" "$LOC/peer_db"
>> +       done
>> +
>> +       echo "  Passed"
>> +}
>> +
>> +function read_spad()
>> +{
>> +       VPATH=$1
>> +       IDX=$2
>> +
>> +       ROW=($(read_file "$VPATH" | grep -e "^$IDX"))
>> +       let VAL=${ROW[1]} || true
>> +       echo $VAL
>> +}
>> +
>> +function scratchpad_test()
>> +{
>> +       LOC=$1
>> +       REM=$2
>> +       CNT=$(read_file "$LOC/spad" | wc -l)
>> +
>> +       echo "Running spad tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +       for ((i = 0; i < $CNT; i++)); do
>> +               VAL=$RANDOM
>> +               write_file "$i $VAL" "$LOC/peer_spad"
>> +               RVAL=$(read_spad "$REM/spad" $i)
>> +
>> +               if [[ "$VAL" != "$RVAL" ]]; then
>> +                       echo "Scratchpad doesn't match expected value $VAL " \
>> +                            "in $REM/spad, got $RVAL" >&2
>> +                       exit -1
>> +               fi
>> +
>> +       done
>> +
>> +       echo "  Passed"
>> +}
>> +
>> +function write_mw()
>> +{
>> +       split_remote $2
>> +
>> +       if [[ "$REMOTE" != "" ]]; then
>> +               ssh "$REMOTE" \
>> +                       dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
>> +       else
>> +               dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
>> +       fi
>> +}
>> +
>> +function mw_test()
>> +{
>> +       IDX=$1
>> +       LOC=$2
>> +       REM=$3
>> +
>> +       echo "Running $IDX tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +       write_mw "$LOC/$IDX"
>> +
>> +       split_remote "$LOC/$IDX"
>> +       if [[ "$REMOTE" == "" ]]; then
>> +               A=$VPATH
>> +       else
>> +               A=/tmp/ntb_test.$$.A
>> +               ssh "$REMOTE" cat "$VPATH" > "$A"
>> +       fi
>> +
>> +       split_remote "$REM/peer_$IDX"
>> +       if [[ "$REMOTE" == "" ]]; then
>> +               B=$VPATH
>> +       else
>> +               B=/tmp/ntb_test.$$.B
>> +               ssh "$REMOTE" cat "$VPATH" > "$B"
>> +       fi
>> +
>> +       cmp "$A" "$B"
>> +       if [[ $? != 0 ]]; then
>> +               echo "Memory window $MW did not match!" >&2
>> +       fi
>> +
>> +       if [[ "$A" == "/tmp/*" ]]; then
>> +               rm "$A"
>> +       fi
>> +
>> +       if [[ "$B" == "/tmp/*" ]]; then
>> +               rm "$B"
>> +       fi
>> +
>> +       echo "  Passed"
>> +}
>> +
>> +function pingpong_test()
>> +{
>> +       LOC=$1
>> +       REM=$2
>> +
>> +       echo "Running ping pong tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +       LOC_START=$(read_file $LOC/count)
>> +       REM_START=$(read_file $REM/count)
>> +
>> +       sleep 7
>> +
>> +       LOC_END=$(read_file $LOC/count)
>> +       REM_END=$(read_file $REM/count)
>> +
>> +       if [[ $LOC_START == $LOC_END ]] || [[ $REM_START == $REM_END ]]; then
>> +               echo "Ping pong counter not incrementing!" >&2
>> +               exit 1
>> +       fi
>> +
>> +       echo "  Passed"
>> +}
>> +
>> +function perf_test()
>> +{
>> +       USE_DMA=$1
>> +
>> +       if [[ $USE_DMA == "1" ]]; then
>> +               WITH="with"
>> +       else
>> +               WITH="without"
>> +       fi
>> +
>> +       _modprobe ntb_perf run_order=$PERF_RUN_ORDER \
>> +               max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA
>> +
>> +       echo "Running local perf test $WITH DMA"
>> +       write_file "" $LOCAL_PERF/run
>> +       echo -n "  "
>> +       read_file $LOCAL_PERF/run
>> +       echo "  Passed"
>> +
>> +       echo "Running remote perf test $WITH DMA"
>> +       write_file "" $REMOTE_PERF/run
>> +       echo -n "  "
>> +       read_file $LOCAL_PERF/run
>> +       echo "  Passed"
>> +
>> +       _modprobe -r ntb_perf
>> +}
>> +
>> +function ntb_tool_tests()
>> +{
>> +       LOCAL_TOOL=$DEBUGFS/ntb_tool/$LOCAL_DEV
>> +       REMOTE_TOOL=$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV
>> +
>> +       echo "Starting ntb_tool tests..."
>> +
>> +       _modprobe ntb_tool
>> +
>> +       echo > $LOCAL_TOOL/link
>> +
>> +       doorbell_test $LOCAL_TOOL $REMOTE_TOOL
>> +       doorbell_test $REMOTE_TOOL $LOCAL_TOOL
>> +       scratchpad_test $LOCAL_TOOL $REMOTE_TOOL
>> +       scratchpad_test $REMOTE_TOOL $LOCAL_TOOL
>> +
>> +       for MW in $(ls $LOCAL_TOOL/mw*); do
>> +               MW=$(basename $MW)
>> +               mw_test $MW $LOCAL_TOOL $REMOTE_TOOL
>> +               mw_test $MW $REMOTE_TOOL $LOCAL_TOOL
>> +       done
>> +
>> +       _modprobe -r ntb_tool
>> +}
>> +
>> +function ntb_pingpong_tests()
>> +{
>> +       LOCAL_PP=$DEBUGFS/ntb_pingpong/$LOCAL_DEV
>> +       REMOTE_PP=$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV
>> +
>> +       echo "Starting ntb_pingpong tests..."
>> +
>> +       _modprobe ntb_pingpong
>> +
>> +       pingpong_test $LOCAL_PP $REMOTE_PP
>> +
>> +       _modprobe -r ntb_pingpong
>> +}
>> +
>> +function ntb_perf_tests()
>> +{
>> +       LOCAL_PERF=$DEBUGFS/ntb_perf/$LOCAL_DEV
>> +       REMOTE_PERF=$REMOTE_HOST:$DEBUGFS/ntb_perf/$REMOTE_DEV
>> +
>> +       echo "Starting ntb_perf tests..."
>> +
>> +       perf_test 0
>> +
>> +       if [[ $RUN_DMA_TESTS ]]; then
>> +               perf_test 1
>> +       fi
>> +}
>> +
>> +function cleanup()
>> +{
>> +       set +e
>> +       _modprobe -r ntb_tool 2> /dev/null
>> +       _modprobe -r ntb_perf 2> /dev/null
>> +       _modprobe -r ntb_pingpong 2> /dev/null
>> +       _modprobe -r ntb_transport 2> /dev/null
>> +       set -e
>> +}
>> +
>> +cleanup
>> +
>> +if ! [[ $$DONT_CLEANUP ]]; then
>> +       trap cleanup EXIT
>> +fi
>> +
>> +if [ "$(id -u)" != "0" ]; then
>> +       echo "This script must be run as root" 1>&2
>> +       exit 1
>> +fi
>> +
>> +if [[ "$LIST_DEVS" == TRUE ]]; then
>> +       _modprobe ntb_tool
>> +       echo "Local Devices:"
>> +       ls -1 /sys/kernel/debug/ntb_tool
>> +       echo
>> +
>> +       if [[ "$REMOTE_HOST" != "" ]]; then
>> +               echo "Remote Devices:"
>> +               ssh $REMOTE_HOST ls -1 /sys/kernel/debug/ntb_tool
>> +       fi
>> +
>> +       _modprobe -r ntb_tool
>> +
>> +       exit 0
>> +fi
>> +
>> +if [[ "$LOCAL_DEV" == $"" ]] || [[ "$REMOTE_DEV" == $"" ]]; then
>> +       show_help
>> +       exit 1
>> +fi
>> +
>> +ntb_tool_tests
>> +echo
>> +ntb_pingpong_tests
>> +echo
>> +ntb_perf_tests
>> +echo
>> --
>> 2.1.4
>>

Re: [PATCH 8/8] ntb_test: Add a selftest script for the NTB subsystem

[Logan Gunthorpe]

On 14/06/16 08:16 AM, Shuah Khan wrote:
> On 06/14/2016 08:06 AM, Jon Mason wrote:
>> On Fri, Jun 10, 2016 at 6:54 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>>> This script automates testing doorbells, scratchpads and memory windows
>>> for an NTB device. It can be run locally, with the NTB looped
>>> back to the same host or use SSH to remotely control the second host.
>>>
>>> In the single host case, the script just needs to be passed two
>>> arguments: a PCI ID for each side of the link. In the two host case
>>> the -r option must be used to specify the remote hostname (which must
>>> be SSH accessible and should probably have ssh-keys exchanged).
>>
>> I appreciate the work that you are putting in here, but test shell
>> scripts are not accepted into the kernel source.

Yeah, I wasn't aware of this rule. I previously did some work on a very
similar shell script for NVM fabrics. Though that hasn't made it to
upstream yet.

> I don't see any reason for this script to be not part of kernel selftests.
> I think it will be a good addition. We probably don't want to include it in
> the auto run of the selftest suite.
> Jon! I you would like to take this script through your ntb tree, here is
> my ack for the script for kselftest part.
> 
> Acked-by: Shuah Khan <shuahkh@osg.samsung.com>
> 

Thanks Shauh!

>> I think a better place for this to be shared would be on the github
>> account wiki, https://github.com/jonmason/ntb/wiki
>> In fact, I'd really like for someone to add some pages there on using
>> the ntb tools and testing.  If you are willing, I'd be most
>> appreciative.

I can probably make some time for this later in the week.


Logan

RE: [PATCH 8/8] ntb_test: Add a selftest script for the NTB subsystem

[Allen Hubbe]

From: Logan Gunthorpe
> This script automates testing doorbells, scratchpads and memory windows
> for an NTB device. It can be run locally, with the NTB looped
> back to the same host or use SSH to remotely control the second host.
> 
> In the single host case, the script just needs to be passed two
> arguments: a PCI ID for each side of the link. In the two host case
> the -r option must be used to specify the remote hostname (which must
> be SSH accessible and should probably have ssh-keys exchanged).
> 
> A sample run looks like this:
> 
> $ sudo ./ntb_test.sh 0000:03:00.1 0000:83:00.1 -p 29
> Starting ntb_tool tests...
> Running db tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running db tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
> Running spad tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running spad tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
> Running mw0 tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running mw0 tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
> Running mw1 tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> Running mw1 tests on: 0000:83:00.1 / 0000:03:00.1
>   Passed
> 
> Starting ntb_pingpong tests...
> Running ping pong tests on: 0000:03:00.1 / 0000:83:00.1
>   Passed
> 
> Starting ntb_perf tests...
> Running local perf test without DMA
>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>   Passed
> Running remote perf test without DMA
>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>   Passed
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  MAINTAINERS                             |   1 +
>  tools/testing/selftests/ntb/ntb_test.sh | 386 ++++++++++++++++++++++++++++++++
>  2 files changed, 387 insertions(+)
>  create mode 100755 tools/testing/selftests/ntb/ntb_test.sh
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9c567a4..f178e7e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7846,6 +7846,7 @@ F:	drivers/ntb/
>  F:	drivers/net/ntb_netdev.c
>  F:	include/linux/ntb.h
>  F:	include/linux/ntb_transport.h
> +F:	tools/testing/selftests/ntb/
> 
>  NTB INTEL DRIVER
>  M:	Jon Mason <jdmason@kudzu.us>
> diff --git a/tools/testing/selftests/ntb/ntb_test.sh
> b/tools/testing/selftests/ntb/ntb_test.sh
> new file mode 100755
> index 0000000..e4a89e9
> --- /dev/null
> +++ b/tools/testing/selftests/ntb/ntb_test.sh
> @@ -0,0 +1,386 @@
> +#!/bin/bash
> +# Copyright (c) 2016 Microsemi. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# Author: Logan Gunthorpe <logang@deltatee.com>
> +
> +REMOTE_HOST=
> +LIST_DEVS=FALSE
> +
> +DEBUGFS=${DEBUGFS-/sys/kernel/debug}
> +
> +PERF_RUN_ORDER=32
> +MAX_MW_SIZE=0
> +RUN_DMA_TESTS=
> +DONT_CLEANUP=
> +
> +function show_help()
> +{
> +	echo "Usage: $0 [OPTIONS] LOCAL_DEV REMOTE_DEV"
> +	echo "Run tests on a pair of NTB endpoints."
> +	echo
> +	echo "If the NTB device loops back to the same host then,"
> +	echo "just specifying the two PCI ids on the command line is"
> +	echo "sufficient. Otherwise, if the NTB link spans two hosts"
> +	echo "use the -r option to specify the hostname for the remote"
> +	echo "device. SSH will then be used to test the remote side."
> +	echo "An SSH key between the root users of the host would then"
> +	echo "be highly recommended."
> +	echo
> +	echo "Options:"
> +	echo "  -C              don't cleanup ntb modules on exit"
> +	echo "  -d              run dma tests"
> +	echo "  -h              show this help message"
> +	echo "  -l              list available local and remote PCI ids"
> +	echo "  -r REMOTE_HOST  specify the remote's hostname to connect"
> +        echo "                  to for the test (using ssh)"
> +	echo "  -p NUM          ntb_perf run order (default: $PERF_RUN_ORDER)"
> +	echo "  -w max_mw_size  maxmium memory window size"
> +	echo
> +}
> +
> +function parse_args()
> +{
> +	OPTIND=0
> +	while getopts "Cdhlr:p:w:" opt; do
> +		case "$opt" in
> +		C)  DONT_CLEANUP=1 ;;
> +		d)  RUN_DMA_TESTS=1 ;;
> +		h)  show_help; exit 0 ;;
> +		l)  LIST_DEVS=TRUE ;;
> +		r)  REMOTE_HOST=${OPTARG} ;;
> +		p)  PERF_RUN_ORDER=${OPTARG} ;;
> +		w)  MAX_MW_SIZE=${OPTARG} ;;
> +		\?)
> +		    echo "Invalid option: -$OPTARG" >&2
> +		    exit 1
> +		    ;;
> +		esac
> +	done
> +}
> +
> +parse_args "$@"
> +shift $((OPTIND-1))
> +LOCAL_DEV=$1
> +shift
> +parse_args "$@"
> +shift $((OPTIND-1))
> +REMOTE_DEV=$1
> +shift
> +parse_args "$@"
> +
> +set -e
> +
> +function _modprobe()
> +{
> +        modprobe "$@"
> +}
> +
> +function split_remote()
> +{
> +	VPATH=$1
> +	REMOTE=
> +
> +	if [[ "$VPATH" == *":/"* ]]; then
> +		REMOTE=${VPATH%%:*}
> +		VPATH=${VPATH#*:}
> +	fi
> +}
> +
> +function read_file()
> +{
> +	split_remote $1
> +	if [[ "$REMOTE" != "" ]]; then
> +		ssh "$REMOTE" cat "$VPATH"
> +	else
> +		cat "$VPATH"
> +	fi
> +}
> +
> +function write_file()
> +{
> +	split_remote $2
> +	VALUE=$1
> +
> +	if [[ "$REMOTE" != "" ]]; then
> +		ssh "$REMOTE" "echo \"$VALUE\" > \"$VPATH\""
> +	else
> +		echo "$VALUE" > "$VPATH"
> +	fi
> +}
> +
> +function doorbell_test()
> +{
> +	LOC=$1
> +	REM=$2
> +	EXP=0
> +
> +	echo "Running db tests on: $(basename $LOC) / $(basename $REM)"
> +
> +	write_file "c 0xFFFFFFFF" "$REM/db"
> +
> +	for ((i=1; i <= 8; i++)); do
> +		let DB=$(read_file "$REM/db") || true
> +		if [[ "$DB" != "$EXP" ]]; then
> +			echo "Doorbell doesn't match expected value $EXP " \
> +			     "in $REM/db" >&2
> +			exit -1
> +		fi
> +
> +		let "MASK=1 << ($i-1)" || true
> +		let "EXP=$EXP | $MASK" || true
> +		write_file "s $MASK" "$LOC/peer_db"
> +	done
> +
> +	echo "  Passed"
> +}
> +
> +function read_spad()
> +{
> +       VPATH=$1
> +       IDX=$2
> +
> +       ROW=($(read_file "$VPATH" | grep -e "^$IDX"))
> +       let VAL=${ROW[1]} || true
> +       echo $VAL
> +}
> +
> +function scratchpad_test()
> +{
> +	LOC=$1
> +	REM=$2
> +	CNT=$(read_file "$LOC/spad" | wc -l)
> +
> +	echo "Running spad tests on: $(basename $LOC) / $(basename $REM)"
> +
> +	for ((i = 0; i < $CNT; i++)); do
> +		VAL=$RANDOM
> +		write_file "$i $VAL" "$LOC/peer_spad"
> +		RVAL=$(read_spad "$REM/spad" $i)
> +
> +		if [[ "$VAL" != "$RVAL" ]]; then
> +			echo "Scratchpad doesn't match expected value $VAL " \
> +			     "in $REM/spad, got $RVAL" >&2
> +			exit -1
> +		fi
> +
> +	done
> +
> +	echo "  Passed"
> +}
> +
> +function write_mw()
> +{
> +	split_remote $2
> +
> +	if [[ "$REMOTE" != "" ]]; then
> +		ssh "$REMOTE" \
> +			dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
> +	else
> +		dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
> +	fi
> +}
> +
> +function mw_test()
> +{
> +	IDX=$1
> +	LOC=$2
> +	REM=$3
> +
> +	echo "Running $IDX tests on: $(basename $LOC) / $(basename $REM)"
> +
> +	write_mw "$LOC/$IDX"
> +
> +	split_remote "$LOC/$IDX"
> +	if [[ "$REMOTE" == "" ]]; then
> +		A=$VPATH
> +	else
> +		A=/tmp/ntb_test.$$.A
> +		ssh "$REMOTE" cat "$VPATH" > "$A"
> +	fi
> +
> +	split_remote "$REM/peer_$IDX"
> +	if [[ "$REMOTE" == "" ]]; then
> +		B=$VPATH
> +	else
> +		B=/tmp/ntb_test.$$.B
> +		ssh "$REMOTE" cat "$VPATH" > "$B"
> +	fi
> +
> +	cmp "$A" "$B"
> +	if [[ $? != 0 ]]; then
> +		echo "Memory window $MW did not match!" >&2
> +	fi
> +
> +	if [[ "$A" == "/tmp/*" ]]; then
> +		rm "$A"
> +	fi
> +
> +	if [[ "$B" == "/tmp/*" ]]; then
> +		rm "$B"
> +	fi
> +
> +	echo "  Passed"
> +}
> +
> +function pingpong_test()
> +{
> +	LOC=$1
> +	REM=$2
> +
> +	echo "Running ping pong tests on: $(basename $LOC) / $(basename $REM)"
> +
> +	LOC_START=$(read_file $LOC/count)
> +	REM_START=$(read_file $REM/count)
> +
> +	sleep 7
> +
> +	LOC_END=$(read_file $LOC/count)
> +	REM_END=$(read_file $REM/count)
> +
> +	if [[ $LOC_START == $LOC_END ]] || [[ $REM_START == $REM_END ]]; then
> +		echo "Ping pong counter not incrementing!" >&2
> +		exit 1
> +	fi
> +
> +	echo "  Passed"
> +}
> +
> +function perf_test()
> +{
> +	USE_DMA=$1
> +
> +	if [[ $USE_DMA == "1" ]]; then
> +		WITH="with"
> +	else
> +		WITH="without"
> +	fi
> +
> +	_modprobe ntb_perf run_order=$PERF_RUN_ORDER \
> +		max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA
> +
> +	echo "Running local perf test $WITH DMA"
> +	write_file "" $LOCAL_PERF/run
> +	echo -n "  "
> +	read_file $LOCAL_PERF/run
> +	echo "  Passed"
> +
> +	echo "Running remote perf test $WITH DMA"
> +	write_file "" $REMOTE_PERF/run
> +	echo -n "  "
> +	read_file $LOCAL_PERF/run
> +	echo "  Passed"
> +
> +	_modprobe -r ntb_perf
> +}
> +
> +function ntb_tool_tests()
> +{
> +	LOCAL_TOOL=$DEBUGFS/ntb_tool/$LOCAL_DEV
> +	REMOTE_TOOL=$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV
> +
> +	echo "Starting ntb_tool tests..."
> +
> +	_modprobe ntb_tool
> +
> +	echo > $LOCAL_TOOL/link
> +
> +	doorbell_test $LOCAL_TOOL $REMOTE_TOOL
> +	doorbell_test $REMOTE_TOOL $LOCAL_TOOL
> +	scratchpad_test $LOCAL_TOOL $REMOTE_TOOL
> +	scratchpad_test $REMOTE_TOOL $LOCAL_TOOL
> +
> +	for MW in $(ls $LOCAL_TOOL/mw*); do
> +		MW=$(basename $MW)
> +		mw_test $MW $LOCAL_TOOL $REMOTE_TOOL
> +		mw_test $MW $REMOTE_TOOL $LOCAL_TOOL
> +	done
> +
> +	_modprobe -r ntb_tool
> +}
> +
> +function ntb_pingpong_tests()
> +{
> +	LOCAL_PP=$DEBUGFS/ntb_pingpong/$LOCAL_DEV
> +	REMOTE_PP=$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV
> +
> +	echo "Starting ntb_pingpong tests..."
> +
> +	_modprobe ntb_pingpong
> +
> +	pingpong_test $LOCAL_PP $REMOTE_PP
> +
> +	_modprobe -r ntb_pingpong
> +}
> +
> +function ntb_perf_tests()
> +{
> +	LOCAL_PERF=$DEBUGFS/ntb_perf/$LOCAL_DEV
> +	REMOTE_PERF=$REMOTE_HOST:$DEBUGFS/ntb_perf/$REMOTE_DEV
> +
> +	echo "Starting ntb_perf tests..."
> +
> +	perf_test 0
> +
> +	if [[ $RUN_DMA_TESTS ]]; then
> +		perf_test 1
> +	fi
> +}
> +
> +function cleanup()
> +{
> +	set +e
> +	_modprobe -r ntb_tool 2> /dev/null
> +	_modprobe -r ntb_perf 2> /dev/null
> +	_modprobe -r ntb_pingpong 2> /dev/null
> +	_modprobe -r ntb_transport 2> /dev/null
> +	set -e
> +}
> +
> +cleanup
> +
> +if ! [[ $$DONT_CLEANUP ]]; then
> +	trap cleanup EXIT
> +fi
> +
> +if [ "$(id -u)" != "0" ]; then
> +	echo "This script must be run as root" 1>&2
> +	exit 1
> +fi
> +
> +if [[ "$LIST_DEVS" == TRUE ]]; then
> +	_modprobe ntb_tool
> +	echo "Local Devices:"
> +	ls -1 /sys/kernel/debug/ntb_tool
> +	echo

Without loading ntb_tool, would it be sufficient for your tests to do this:

ls -1 /sys/bus/ntb/devices

> +
> +	if [[ "$REMOTE_HOST" != "" ]]; then
> +		echo "Remote Devices:"
> +		ssh $REMOTE_HOST ls -1 /sys/kernel/debug/ntb_tool
> +	fi
> +
> +	_modprobe -r ntb_tool
> +
> +	exit 0
> +fi
> +
> +if [[ "$LOCAL_DEV" == $"" ]] || [[ "$REMOTE_DEV" == $"" ]]; then
> +	show_help
> +	exit 1
> +fi
> +
> +ntb_tool_tests
> +echo
> +ntb_pingpong_tests
> +echo
> +ntb_perf_tests
> +echo
> --
> 2.1.4

Re: [PATCH 8/8] ntb_test: Add a selftest script for the NTB subsystem

[Logan Gunthorpe]

On 14/06/16 09:47 AM, Allen Hubbe wrote:
> From: Logan Gunthorpe
>> This script automates testing doorbells, scratchpads and memory windows
>> for an NTB device. It can be run locally, with the NTB looped
>> back to the same host or use SSH to remotely control the second host.
>>
>> In the single host case, the script just needs to be passed two
>> arguments: a PCI ID for each side of the link. In the two host case
>> the -r option must be used to specify the remote hostname (which must
>> be SSH accessible and should probably have ssh-keys exchanged).
>>
>> A sample run looks like this:
>>
>> $ sudo ./ntb_test.sh 0000:03:00.1 0000:83:00.1 -p 29
>> Starting ntb_tool tests...
>> Running db tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running db tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>> Running spad tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running spad tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>> Running mw0 tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running mw0 tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>> Running mw1 tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>> Running mw1 tests on: 0000:83:00.1 / 0000:03:00.1
>>   Passed
>>
>> Starting ntb_pingpong tests...
>> Running ping pong tests on: 0000:03:00.1 / 0000:83:00.1
>>   Passed
>>
>> Starting ntb_perf tests...
>> Running local perf test without DMA
>>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>>   Passed
>> Running remote perf test without DMA
>>   0: copied 536870912 bytes in 238205 usecs, 2253 MBytes/s
>>   Passed
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> ---
>>  MAINTAINERS                             |   1 +
>>  tools/testing/selftests/ntb/ntb_test.sh | 386 ++++++++++++++++++++++++++++++++
>>  2 files changed, 387 insertions(+)
>>  create mode 100755 tools/testing/selftests/ntb/ntb_test.sh
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 9c567a4..f178e7e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7846,6 +7846,7 @@ F:	drivers/ntb/
>>  F:	drivers/net/ntb_netdev.c
>>  F:	include/linux/ntb.h
>>  F:	include/linux/ntb_transport.h
>> +F:	tools/testing/selftests/ntb/
>>
>>  NTB INTEL DRIVER
>>  M:	Jon Mason <jdmason@kudzu.us>
>> diff --git a/tools/testing/selftests/ntb/ntb_test.sh
>> b/tools/testing/selftests/ntb/ntb_test.sh
>> new file mode 100755
>> index 0000000..e4a89e9
>> --- /dev/null
>> +++ b/tools/testing/selftests/ntb/ntb_test.sh
>> @@ -0,0 +1,386 @@
>> +#!/bin/bash
>> +# Copyright (c) 2016 Microsemi. All Rights Reserved.
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation; either version 2 of
>> +# the License, or (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# Author: Logan Gunthorpe <logang@deltatee.com>
>> +
>> +REMOTE_HOST=
>> +LIST_DEVS=FALSE
>> +
>> +DEBUGFS=${DEBUGFS-/sys/kernel/debug}
>> +
>> +PERF_RUN_ORDER=32
>> +MAX_MW_SIZE=0
>> +RUN_DMA_TESTS=
>> +DONT_CLEANUP=
>> +
>> +function show_help()
>> +{
>> +	echo "Usage: $0 [OPTIONS] LOCAL_DEV REMOTE_DEV"
>> +	echo "Run tests on a pair of NTB endpoints."
>> +	echo
>> +	echo "If the NTB device loops back to the same host then,"
>> +	echo "just specifying the two PCI ids on the command line is"
>> +	echo "sufficient. Otherwise, if the NTB link spans two hosts"
>> +	echo "use the -r option to specify the hostname for the remote"
>> +	echo "device. SSH will then be used to test the remote side."
>> +	echo "An SSH key between the root users of the host would then"
>> +	echo "be highly recommended."
>> +	echo
>> +	echo "Options:"
>> +	echo "  -C              don't cleanup ntb modules on exit"
>> +	echo "  -d              run dma tests"
>> +	echo "  -h              show this help message"
>> +	echo "  -l              list available local and remote PCI ids"
>> +	echo "  -r REMOTE_HOST  specify the remote's hostname to connect"
>> +        echo "                  to for the test (using ssh)"
>> +	echo "  -p NUM          ntb_perf run order (default: $PERF_RUN_ORDER)"
>> +	echo "  -w max_mw_size  maxmium memory window size"
>> +	echo
>> +}
>> +
>> +function parse_args()
>> +{
>> +	OPTIND=0
>> +	while getopts "Cdhlr:p:w:" opt; do
>> +		case "$opt" in
>> +		C)  DONT_CLEANUP=1 ;;
>> +		d)  RUN_DMA_TESTS=1 ;;
>> +		h)  show_help; exit 0 ;;
>> +		l)  LIST_DEVS=TRUE ;;
>> +		r)  REMOTE_HOST=${OPTARG} ;;
>> +		p)  PERF_RUN_ORDER=${OPTARG} ;;
>> +		w)  MAX_MW_SIZE=${OPTARG} ;;
>> +		\?)
>> +		    echo "Invalid option: -$OPTARG" >&2
>> +		    exit 1
>> +		    ;;
>> +		esac
>> +	done
>> +}
>> +
>> +parse_args "$@"
>> +shift $((OPTIND-1))
>> +LOCAL_DEV=$1
>> +shift
>> +parse_args "$@"
>> +shift $((OPTIND-1))
>> +REMOTE_DEV=$1
>> +shift
>> +parse_args "$@"
>> +
>> +set -e
>> +
>> +function _modprobe()
>> +{
>> +        modprobe "$@"
>> +}
>> +
>> +function split_remote()
>> +{
>> +	VPATH=$1
>> +	REMOTE=
>> +
>> +	if [[ "$VPATH" == *":/"* ]]; then
>> +		REMOTE=${VPATH%%:*}
>> +		VPATH=${VPATH#*:}
>> +	fi
>> +}
>> +
>> +function read_file()
>> +{
>> +	split_remote $1
>> +	if [[ "$REMOTE" != "" ]]; then
>> +		ssh "$REMOTE" cat "$VPATH"
>> +	else
>> +		cat "$VPATH"
>> +	fi
>> +}
>> +
>> +function write_file()
>> +{
>> +	split_remote $2
>> +	VALUE=$1
>> +
>> +	if [[ "$REMOTE" != "" ]]; then
>> +		ssh "$REMOTE" "echo \"$VALUE\" > \"$VPATH\""
>> +	else
>> +		echo "$VALUE" > "$VPATH"
>> +	fi
>> +}
>> +
>> +function doorbell_test()
>> +{
>> +	LOC=$1
>> +	REM=$2
>> +	EXP=0
>> +
>> +	echo "Running db tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +	write_file "c 0xFFFFFFFF" "$REM/db"
>> +
>> +	for ((i=1; i <= 8; i++)); do
>> +		let DB=$(read_file "$REM/db") || true
>> +		if [[ "$DB" != "$EXP" ]]; then
>> +			echo "Doorbell doesn't match expected value $EXP " \
>> +			     "in $REM/db" >&2
>> +			exit -1
>> +		fi
>> +
>> +		let "MASK=1 << ($i-1)" || true
>> +		let "EXP=$EXP | $MASK" || true
>> +		write_file "s $MASK" "$LOC/peer_db"
>> +	done
>> +
>> +	echo "  Passed"
>> +}
>> +
>> +function read_spad()
>> +{
>> +       VPATH=$1
>> +       IDX=$2
>> +
>> +       ROW=($(read_file "$VPATH" | grep -e "^$IDX"))
>> +       let VAL=${ROW[1]} || true
>> +       echo $VAL
>> +}
>> +
>> +function scratchpad_test()
>> +{
>> +	LOC=$1
>> +	REM=$2
>> +	CNT=$(read_file "$LOC/spad" | wc -l)
>> +
>> +	echo "Running spad tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +	for ((i = 0; i < $CNT; i++)); do
>> +		VAL=$RANDOM
>> +		write_file "$i $VAL" "$LOC/peer_spad"
>> +		RVAL=$(read_spad "$REM/spad" $i)
>> +
>> +		if [[ "$VAL" != "$RVAL" ]]; then
>> +			echo "Scratchpad doesn't match expected value $VAL " \
>> +			     "in $REM/spad, got $RVAL" >&2
>> +			exit -1
>> +		fi
>> +
>> +	done
>> +
>> +	echo "  Passed"
>> +}
>> +
>> +function write_mw()
>> +{
>> +	split_remote $2
>> +
>> +	if [[ "$REMOTE" != "" ]]; then
>> +		ssh "$REMOTE" \
>> +			dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
>> +	else
>> +		dd if=/dev/urandom "of=$VPATH" 2> /dev/null || true
>> +	fi
>> +}
>> +
>> +function mw_test()
>> +{
>> +	IDX=$1
>> +	LOC=$2
>> +	REM=$3
>> +
>> +	echo "Running $IDX tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +	write_mw "$LOC/$IDX"
>> +
>> +	split_remote "$LOC/$IDX"
>> +	if [[ "$REMOTE" == "" ]]; then
>> +		A=$VPATH
>> +	else
>> +		A=/tmp/ntb_test.$$.A
>> +		ssh "$REMOTE" cat "$VPATH" > "$A"
>> +	fi
>> +
>> +	split_remote "$REM/peer_$IDX"
>> +	if [[ "$REMOTE" == "" ]]; then
>> +		B=$VPATH
>> +	else
>> +		B=/tmp/ntb_test.$$.B
>> +		ssh "$REMOTE" cat "$VPATH" > "$B"
>> +	fi
>> +
>> +	cmp "$A" "$B"
>> +	if [[ $? != 0 ]]; then
>> +		echo "Memory window $MW did not match!" >&2
>> +	fi
>> +
>> +	if [[ "$A" == "/tmp/*" ]]; then
>> +		rm "$A"
>> +	fi
>> +
>> +	if [[ "$B" == "/tmp/*" ]]; then
>> +		rm "$B"
>> +	fi
>> +
>> +	echo "  Passed"
>> +}
>> +
>> +function pingpong_test()
>> +{
>> +	LOC=$1
>> +	REM=$2
>> +
>> +	echo "Running ping pong tests on: $(basename $LOC) / $(basename $REM)"
>> +
>> +	LOC_START=$(read_file $LOC/count)
>> +	REM_START=$(read_file $REM/count)
>> +
>> +	sleep 7
>> +
>> +	LOC_END=$(read_file $LOC/count)
>> +	REM_END=$(read_file $REM/count)
>> +
>> +	if [[ $LOC_START == $LOC_END ]] || [[ $REM_START == $REM_END ]]; then
>> +		echo "Ping pong counter not incrementing!" >&2
>> +		exit 1
>> +	fi
>> +
>> +	echo "  Passed"
>> +}
>> +
>> +function perf_test()
>> +{
>> +	USE_DMA=$1
>> +
>> +	if [[ $USE_DMA == "1" ]]; then
>> +		WITH="with"
>> +	else
>> +		WITH="without"
>> +	fi
>> +
>> +	_modprobe ntb_perf run_order=$PERF_RUN_ORDER \
>> +		max_mw_size=$MAX_MW_SIZE use_dma=$USE_DMA
>> +
>> +	echo "Running local perf test $WITH DMA"
>> +	write_file "" $LOCAL_PERF/run
>> +	echo -n "  "
>> +	read_file $LOCAL_PERF/run
>> +	echo "  Passed"
>> +
>> +	echo "Running remote perf test $WITH DMA"
>> +	write_file "" $REMOTE_PERF/run
>> +	echo -n "  "
>> +	read_file $LOCAL_PERF/run
>> +	echo "  Passed"
>> +
>> +	_modprobe -r ntb_perf
>> +}
>> +
>> +function ntb_tool_tests()
>> +{
>> +	LOCAL_TOOL=$DEBUGFS/ntb_tool/$LOCAL_DEV
>> +	REMOTE_TOOL=$REMOTE_HOST:$DEBUGFS/ntb_tool/$REMOTE_DEV
>> +
>> +	echo "Starting ntb_tool tests..."
>> +
>> +	_modprobe ntb_tool
>> +
>> +	echo > $LOCAL_TOOL/link
>> +
>> +	doorbell_test $LOCAL_TOOL $REMOTE_TOOL
>> +	doorbell_test $REMOTE_TOOL $LOCAL_TOOL
>> +	scratchpad_test $LOCAL_TOOL $REMOTE_TOOL
>> +	scratchpad_test $REMOTE_TOOL $LOCAL_TOOL
>> +
>> +	for MW in $(ls $LOCAL_TOOL/mw*); do
>> +		MW=$(basename $MW)
>> +		mw_test $MW $LOCAL_TOOL $REMOTE_TOOL
>> +		mw_test $MW $REMOTE_TOOL $LOCAL_TOOL
>> +	done
>> +
>> +	_modprobe -r ntb_tool
>> +}
>> +
>> +function ntb_pingpong_tests()
>> +{
>> +	LOCAL_PP=$DEBUGFS/ntb_pingpong/$LOCAL_DEV
>> +	REMOTE_PP=$REMOTE_HOST:$DEBUGFS/ntb_pingpong/$REMOTE_DEV
>> +
>> +	echo "Starting ntb_pingpong tests..."
>> +
>> +	_modprobe ntb_pingpong
>> +
>> +	pingpong_test $LOCAL_PP $REMOTE_PP
>> +
>> +	_modprobe -r ntb_pingpong
>> +}
>> +
>> +function ntb_perf_tests()
>> +{
>> +	LOCAL_PERF=$DEBUGFS/ntb_perf/$LOCAL_DEV
>> +	REMOTE_PERF=$REMOTE_HOST:$DEBUGFS/ntb_perf/$REMOTE_DEV
>> +
>> +	echo "Starting ntb_perf tests..."
>> +
>> +	perf_test 0
>> +
>> +	if [[ $RUN_DMA_TESTS ]]; then
>> +		perf_test 1
>> +	fi
>> +}
>> +
>> +function cleanup()
>> +{
>> +	set +e
>> +	_modprobe -r ntb_tool 2> /dev/null
>> +	_modprobe -r ntb_perf 2> /dev/null
>> +	_modprobe -r ntb_pingpong 2> /dev/null
>> +	_modprobe -r ntb_transport 2> /dev/null
>> +	set -e
>> +}
>> +
>> +cleanup
>> +
>> +if ! [[ $$DONT_CLEANUP ]]; then
>> +	trap cleanup EXIT
>> +fi
>> +
>> +if [ "$(id -u)" != "0" ]; then
>> +	echo "This script must be run as root" 1>&2
>> +	exit 1
>> +fi
>> +
>> +if [[ "$LIST_DEVS" == TRUE ]]; then
>> +	_modprobe ntb_tool
>> +	echo "Local Devices:"
>> +	ls -1 /sys/kernel/debug/ntb_tool
>> +	echo
> 
> Without loading ntb_tool, would it be sufficient for your tests to do this:
> 
> ls -1 /sys/bus/ntb/devices

Oh, thanks! I didn't even consider that. I'll make that change.

Logan