Re: [PATCH v2 6/8] ntb_tool: Add link status and files to debugfs

[Logan Gunthorpe]

On 15/06/16 10:02 AM, Allen Hubbe wrote:
>> My understanding is that ntb_tool is really just a test client to verify
>> the API and the hardware. I personally would not recommend it for any
>> real applications. As such, I don't think this philosophical argument
>> really matches that goal.
> 
> The purpose is to "verify the API and the hardware", not to support "real applications."
> 
> The link status reported by the tool should be the link status reported by "the API and the hardware," and not something else that might be convenient for "my ntb_test script" or "anyone else trying to script with ntb_tool."  The primary purpose of ntb_tool is api-level debugging of hardware and drivers, not scripting.
> 
> The problem with races in ntb_tool is due to auto-configuration of memory windows in ntb_tool.  Instead of having ntb_tool setup the memory windows automatically, maybe instead it should provide a file to control the memory windows via debugfs.  Reading the file can format what is returned by ntb_mw_get_range(), and writing the file can allocate a buffer and call ntb_mw_set_trans(), or ntb_mw_clear_trans() and free the buffer.  Then, the test script can wait for link up, then setup the memory windows, and then finally proceed with the rest of the tests, and there would be no race.  There would be no confusion about what "link up" means, and ntb_tool would more closely resemble the ntb.h api for memory windows.

Ok, well this is a good deal more complicated than it is now but I can
live with it. I'll work something up shortly.


> That's interesting about the hardware.  Maybe the driver for that particular hardware should make sure that any translation register programming happens before reporting link up to its clients.  Otherwise, ntb_transport will be broken on that hardware.  The ntb_transport driver configures memory windows the first time the link comes up, and only ever again if a different memory window size is negotiated (unlikely).
> 
> There are two reasons for doing the configuration after link up in ntb_transport.  First, it avoids consuming memory resources if the link never comes up.  Second, ntb_transport negotiates with its peer how much of the memory window will actually be used.  The ntb_perf tool is similar.

Hmm, yes I didn't notice that in ntb_transport. We'll have to make some
considerations for that in our driver.

Logan