[nvmewin] review comments for patch from Google

Thu Jun 9 14:32:18 PDT 2016

Hi Iuliu/Suman,

I’m of the opinion that any solution provided should be as robust as possible. In the case Suman points out, concurrent channels should still be enabled even if the number of cores and queues do not match. This is especially true for large core count systems (>128 logical cpu cores) but devices may only support 32 queues or less.

Thank you…
Ray

From: Iuliu Rus [mailto:iuliur at google.com]
Sent: Thursday, June 09, 2016 2:09 PM
To: suman.p at samsung.com
Cc: Robles, Raymond C <raymond.c.robles at intel.com>; nvmewin at lists.openfabrics.org
Subject: Re: review comments for patch from Google

Which one do you prefer? Our google driver has the synchronization in place already for a different reason. Or, I can follow your second suggestion.

On Thu, Jun 9, 2016 at 7:39 AM, SUMAN PRAKASH B <suman.p at samsung.com<mailto:suman.p at samsung.com>> wrote:

Hi Iuliu,

Please find my review comments below:

1. If the number of Cores and Queues supported by device are not same, then setting STOR_PERF_CONCURRENT_CHANNELS will corrupt the submission queue. For example, if device has 4 queues and connected to target PC which has 8 logical processors, and if STOR_PERF_CONCURRENT_CHANNELS is set, all the 8 logical processors will get startIo calls concurrently. As per OFA driver NUMA design, the IO Queue 1 will be shared by core 0 and core 4. There is possibility that core 0 and core 4 will get startio() calls concurrently and will access/update the listenty and queue 1 doorbell register at same time, which will lead to listentry and doorbell register corruption.
This problem can be address by following ways:
a. Protect the listentry and doorbell before access/update.
b. Do not set STOR_PERF_CONCURRENT_CHANNELS when number of cores and queues are not same.

2. We can use pRMT->NumActiveCores which is updated in NVMeEnumNumaCores() instead of using KeQueryActiveProcessorCountEx/KeQueryActiveProcessorCount.

Please let us know your opinion.

Thanks,
Suman

------- Original Message -------

Sender : Robles, Raymond C<raymond.c.robles at intel.com<mailto:raymond.c.robles at intel.com>>

Date : Jun 09, 2016 03:56 (GMT+05:30)

Title : Re: [nvmewin] Happy New Year... and status update

Thank you Iuliu!

Reviewing companies, please try to target the review completion for 2 weeks from today (6/22/16).

Thank you!
Ray

From: Iuliu Rus [mailto:iuliur at google.com<mailto:iuliur at google.com>]
Sent: Wednesday, June 08, 2016 12:26 PM
To: Robles, Raymond C <raymond.c.robles at intel.com<mailto:raymond.c.robles at intel.com>>
Cc: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: Re: Happy New Year... and status update

I attached our patch.

On Tue, Jun 7, 2016 at 2:37 PM, Iuliu Rus <iuliur at google.com<mailto:iuliur at google.com>> wrote:
Let me see if i can get it done by tomorrow eod...

On Tue, Jun 7, 2016 at 2:31 PM, Robles, Raymond C <raymond.c.robles at intel.com<mailto:raymond.c.robles at intel.com>> wrote:
Hi Iuliu,

That’s great… thank you. Yes, the process is still the same (using SVN patches).

Please let me know if you have any questions/comments. My only question for you is an ETA for the patch to be submitted.

Thank you…
Ray

From: Iuliu Rus [mailto:iuliur at google.com<mailto:iuliur at google.com>]
Sent: Tuesday, June 07, 2016 2:28 PM
To: Robles, Raymond C <raymond.c.robles at intel.com<mailto:raymond.c.robles at intel.com>>
Cc: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: Re: Happy New Year... and status update

Yes we can send that to you. Is the process still the same?

On Tue, Jun 7, 2016 at 1:25 PM, Robles, Raymond C <raymond.c.robles at intel.com<mailto:raymond.c.robles at intel.com>> wrote:
Hello Iuliu,

As the OFA community is closing on its mid-year release, we have only one more patch that was targeted… concurrent channels. Will Google still be submitting the concurrent channels patch?

Thank you…
Ray

From: nvmewin-bounces at lists.openfabrics.org<mailto: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, January 14, 2016 4:55 PM
To: 'nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>' <nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>>
Subject: [nvmewin] Happy New Year... and status update

Hello and Happy New Year,

I hope everyone had a great holiday season and is off to a great start to the new year!

As communicated last month, the patch from HGST for the SCSI multi-initiator changes has been approved and pushed. The holidays slowed down much of the progress on the OFA driver and there were several patches that did not get pushed prior to the end of the year. The list of patches remained to be pushed are as follows…

•         Namespace Management (Intel)

•         Perf Opts (Samsung)

•         Win 8.1 Timers (Samsung)

•         EOL Read Only (Samsung)

•         Concurrent channels (Google)

The namespace management patch will be sent out for review tomorrow (look for the patch email from Carolyn)… stay tuned!!!

However, once this patch is resolved, we as a community will have to make a decision on an official release strategy. The patch process and cadence was significantly slower in 2015 which leaves us with a few options.

1.       Release what is in the trunk today (or after the namespace management patch)… and call that the 2015 release (albeit later than expected)

2.       In lieu of an official 2015 release, we push the remaining patches listed above… and then release in ~Q2 of 2015. Basically skip a 2015 release and go right into the mid-2016 release.

3.       Remove the concept of “official releases” from the OFA Windows NVMe driver and just allow people, companies, and users to pull from the OFA trunk as necessary.

For #3 above, my thoughts are that because the OFA driver is not a production driver… but a reference and enabling driver, it should/could be managed as just that… a reference baseline driver that any potential user can go and grab the source, or contribute a patch. Nothing more… nothing less.

For the release decision, I’ll be happy to call a meeting… but we can also handle it via email as well… just let me know. Feedback from all is welcome… but I would request mandatory feedback form the 4 reviewing companies: Samsung, HGST, PMC-Sierra, and Intel. How would you like to proceed?

Thanks,
Ray

[cid:image001.png at 01CB3870.4BB88E70]
Raymond C. Robles
Non-Volatile Memory Solutions Group
Intel Corporation
Office: 480-554-2600<tel:480-554-2600>
Mobile: 480-399-0645<tel:480-399-0645>

[cid:image002.gif at 01D1C25B.B96EBDB0]

[http://ext.samsung.net/mailcheck/SeenTimeChecker?do=9226f2572c3ad117df6a46883b3f3dde1d283838fa336dda194352576c69fe9bfdd53f331c9856e34bea3c1765014a1208cece8541bc14eacf878f9a26ce15a0]

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20160609/ccc12a64/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.png
Type: image/png
Size: 1756 bytes
Desc: image001.png
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20160609/ccc12a64/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image002.gif
Type: image/gif
Size: 13168 bytes
Desc: image002.gif
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20160609/ccc12a64/attachment.gif>