Message-ID: <82408ae9-5f19-0df6-652c-9ffd5c6d0d8a@deltatee.com>
Date:	Mon, 27 Jun 2016 10:08:55 -0600
From:	Logan Gunthorpe <logang@...tatee.com>
To:	Jon Mason <jdmason@...zu.us>
Cc:	Dave Jiang <dave.jiang@...el.com>,
	Allen Hubbe <Allen.Hubbe@....com>,
	Shuah Khan <shuahkh@....samsung.com>,
	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
	linux-ntb@...glegroups.com, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v5 00/10] NTB Selftest Script

Great! Thanks Jon.

Logan

On 26/06/16 05:29 PM, Jon Mason wrote:
> On Mon, Jun 20, 2016 at 01:15:03PM -0600, Logan Gunthorpe wrote:
>> Sorry, I thought this was done but I found one more minor issue with
>> these patches so I'm resubmitting them one last time. Besides this isuse,
>> I think I have acks for all the patches and everything is working as I'd
>> like.
> 
> Applying patch 05/10 to my ntb branch (as it is a bug fix and should
> go into 4.7)
> 
> Applying the rest to my ntb-next branch (which should go into 4.8)
> 
> Note: I'm including patch 09/10 in my tree, even though it is for the
> selftest subsystem.  Given that Shuah Khan acked it, I assume this is
> the desired outcome.
> 
> Thanks,
> Jon
> 
>>
>> Changes since v4:
>>
>> 1) In patch 0006, when setting a translation fails in tool_setup_mw,
>> we now properly clean up the peer dma memory. Before that patch, it
>> wasn't required because if tool_setup_mw failed the module would clean
>> up all mws. After this patch, the clean up never happened, so it would
>> return an error to the user but the DMA memory would still be allocated
>> and the peer_trans file would report that it's ready but the debugfs
>> file would not have been created. In v5, after an error, no DMA memory
>> is allocated and it will still report that it's not ready to the user when
>> reading peer_trans.
>>
>> ---
>>
>> Changes since v3:
>>
>> 1) Check the error returned when setting the link in ntb_tool and pass
>> it back to the user.
>>
>> 2) If an error occurs when setting the link down during a link test in
>> ntb_test, just print unsupported and continue on. (For hardware that
>> does not support setting the link down.)
>>
>> 3) Fix a race condition problem introduced by clearing the link is
>> up flag in ntb_perf. We do this by getting rid of the link cleanup
>> work and doing the few actions in the link event handler.
>> Dave Jiang has already ok'd this approach.
>>
>> ---
>>
>> Changes since v2:
>>
>> 1) As per Allan's suggestion, I've added a patch to postpone the
>> peer memory window setup until the user requests it with a 'peer_trans*'
>> file. This pushes the ntb_tool API closer to the kernel API and allows
>> the link status file to return the raw status of the NTB link.
>>
>> 2) Change the link status file to return the value of ntb_link_is_up
>> instead of the local 'memory window ready' value.
>>
>> 3) Change the link_event file to just block on write. As it was in v2,
>> if multiple users attempted to use the link_event file they could
>> corrupt the state another user had set. Reads to this file are no
>> longer permitted.
>>
>> 4) Updates to the ntb_test script to accommodate the above changes.
>>
>> 5) Added some link tests to the ntb_test script. It will bring the link
>> down and check that the other side agrees.
>>
>> 6) Added a minor bug fix (Patch #10) to ntb_perf. During discussions
>> with Allen it was noticed that the link_is_up flag is never cleared if
>> the link goes away.
>>
>> ---
>>
>> Changes since v1:
>>
>> 1) Add a comment to explain the *15 in the buf size calculation,
>> as per Allen's feedback.
>>
>> 2) Clean up the changes to the pingpong client as there were some
>> sloppy copying mistakes.
>>
>> 3) Rework the 'link' file in ntb_tool as per Allen's suggestions.
>> I've added a 'link_event' file the works essentially how he's asked.
>> Though, I found no need to use a completion as suggested and the flow
>> is maybe slightly simpler than he's suggested. Just write a boolean
>> to the event file then read to wait for the link to be either up or
>> down. There's still some discussion on the best interface and it's
>> not much work to make additional minor functional changes.
>>
>> 4) Update the selftest script to use the new 'link_event' file.
>>
>> 5) Minor change to the way the selftest script lists devices thanks to
>> Allen's observation.
>>
>> ---
>>
>> I've written a ntb_test.sh script that would probably be useful if it
>> were included in the kernel. This series ends with that script and
>> includes some useful interface improvements and fixes to the existing ntb
>> test modules. Please see each individual commit for more information.
>> They are mostly independent.
>>
>> The series is based off of v4.6 plus the patches I've submitted that
>> have been accepted into ntb-next. They've been run through checkpatch
>> with --strict this time.
>>
>> As always, I'm happy to incorporate any feedback.
>>
>> Thanks,
>>
>> Logan
>>
>> ---
>>
>> Logan Gunthorpe (10):
>>   ntb_perf: Schedule based on time not on performance
>>   ntb_perf: Improve thread handling to increase robustness
>>   ntb_perf: Return results by reading the run file
>>   ntb_perf: Wait for link before running test
>>   ntb_tool: BUG: Ensure the buffer size is large enough to return all
>>     spads
>>   ntb_tool: Postpone memory window initialization for the user
>>   ntb_tool: Add link status and files to debugfs
>>   ntb_pingpong: Add a debugfs file to get the ping count
>>   ntb_test: Add a selftest script for the NTB subsystem
>>   ntb_perf: clear link_is_up flag when the link goes down.
>>
>>  MAINTAINERS                             |   1 +
>>  drivers/ntb/test/ntb_perf.c             | 224 +++++++++------
>>  drivers/ntb/test/ntb_pingpong.c         |  62 ++++-
>>  drivers/ntb/test/ntb_tool.c             | 468 ++++++++++++++++++++++----------
>>  tools/testing/selftests/ntb/ntb_test.sh | 422 ++++++++++++++++++++++++++++
>>  5 files changed, 957 insertions(+), 220 deletions(-)
>>  create mode 100755 tools/testing/selftests/ntb/ntb_test.sh
>>
>> --
>> 2.1.4

Powered by blists - more mailing lists