[nvmewin] patch submission

[Robles, Raymond C]

Hi,

Thanks for the response.

My biggest concern here is how this change will work on currently released NVMe products in the market (i.e. Intel, Samsung, etc.). Do you have a means of unit testing with other NVMe devices?

This fix seems very specific to your “proprietary” NVMe device. No other IHV for NVMe devices is using a similar power methodology. Based on this alone, I’m inclined to not include it in the OFA baseline. However, please keep in mind that the OFA driver is a “reference” driver, and not a production driver (i.e. Linux NVMe driver).

Therefore, your company can take this driver and modify it as you need and release under the FreeBSD license with your H/W. Many IHV vendors do this. In the end, this patch does not “need” to be in the OFA baseline… as it only resolves an issue on your company’s NVMe controller.

For now, I think the next steps are:


-          Get feedback from rest of OFA NVMeWin reviewing companies on such a fix

-          Test your patch on other 3rd party NVMe drives

Thanks,
Ray

From: Yizhong Zhang [mailto:yizhong.zhang at memblaze.com]
Sent: Monday, August 10, 2015 1:21 AM
To: Wenbo Wang; Robles, Raymond C
Subject: 答复: patch submission

Hi Ray,

Thanks for your information and sorry for delayed response. My name is Yizhong and I will track this effort. I am a rookie in NVMe Windows driver, so please correct me if there is anything wrong in my statement.

Cheng already left company so I can only talked with other guys for some internals. Here is the problem background: we have a special NVMe test setup. We have a self-made NVMe card slot, which separates PCIe signals from power. In such way, the NVMe card gets power with a separated power module, instead of power from PCIe slot. We found a bug in such setup. If we unplug the power cable and plug the power cable again, we observed Windows hang (no any response). So we add the patch later, and it proves the bug is fixed.

I looked into code and found the passive wait in NVMe reset path (NVMeInitialize and NVMeResetAdapter), but I didn’t see NVMeWaitForCtrlRDY  been called in NVMeEnableAdapter. So this fix make sense in my opinion. The side effect is driver may spends ~1s (estimated) to wait for controller ready after power recover.

We only test the fix with the bug reproduce steps. There is no unit test here. As I mentioned above, it should be safe for all other functionalities except extra delay during NVMe initialization stage.

Could you please let us know your further advice? Thanks in advance.

Thanks,
-yz

发件人: Wenbo Wang
发送时间: Saturday, August 8, 2015 11:43 PM
收件人: raymond.c.robles at intel.com<mailto:raymond.c.robles at intel.com>
抄送: Yizhong Zhang
主题: FW: patch submission
重要性: 高

Hi Ray,
Thanks for your email.
Due to some changes, Cheng will not work on this OFA driver any more. I and Yizhong will be doing that.
We need to take some time to understand all the history and after that we will response ASAP. Sorry for the inconvenience.
Thanks,
-Wenbo
________________________________
发件人: Robles, Raymond C <raymond.c.robles at intel.com<mailto:raymond.c.robles at intel.com>>
发送时间: 2015年8月7日 3:34
收件人: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>; Cheng Peng
主题: RE: patch submission


Hi Cheng,



I have not seen a response from you on my question below. In order for your patch to be accepted, you must respond to any and all questions/comments from the reviewing companies… as well as from the reflector list. If you do not respond within a timely fashion, your patch will not be considered for integration to the OFA driver. Please respond at your earliest convenience.



Reviewing Companies,



Any feedback, questions, or test results on the patch submission below? The deadline for review is August 12th. Thanks!



Thanks,

Ray Robles



From: nvmewin-bounces at lists.openfabrics.org<mailto:nvmewin-bounces at lists.openfabrics.org> [mailto:nvmewin-bounces at lists.openfabrics.org] On Behalf Of Robles, Raymond C
Sent: Friday, July 31, 2015 11:37 AM
To: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>; Cheng Peng
Subject: Re: [nvmewin] patch submission



Hi Cheng,



I’ve reviewed your code and have some questions.



It looks like NVMeWaitForCtrlRDY is added in init and reset path.  Previously controller ready wait is  performed in passive init path. I did not see any benefit of adding NVMeWaitForCtrlRDY in init path (I feel, old passive init path wait will be right approach to have minimal processor usage).



Could you please provide an explanation on why you moved wait for controller ready to init path?



Thanks,

Ray



From: nvmewin-bounces at lists.openfabrics.org<mailto:nvmewin-bounces at lists.openfabrics.org> [mailto:nvmewin-bounces at lists.openfabrics.org] On Behalf Of Robles, Raymond C
Sent: Thursday, July 30, 2015 3:24 PM
To: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>; Cheng Peng
Subject: Re: [nvmewin] patch submission



Reviewing Companies,



Please provide any review comments and testing results before August 12th. We also have a patch from ULINK Technology after this patch.



In the meantime, Cheng could you please provide the following information about your patch:



-          Reason fix was needed.

-          Files modified and what was modified in each file.

-          How you unit tested your fix.



Thanks,

Ray



From: nvmewin-bounces at lists.openfabrics.org<mailto:nvmewin-bounces at lists.openfabrics.org> [mailto:nvmewin-bounces at lists.openfabrics.org] On Behalf Of Cheng Peng
Sent: Monday, July 06, 2015 10:32 PM
To: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: [nvmewin] patch submission



fix: Wait for device ready when enable adapter



It had been tested on Windows 2008 R2\Windows 2012 x64 platform



Please review it, and if you think it OK, please merge it to SVN trunk, thank you



Btw, the password of ZIP is ofa_nvme
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20150813/45a19146/attachment.html>