[nvmewin] review comments for patch from Google

Thu Jun 9 14:09:09 PDT 2016

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> 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>
>
> *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]
> *Sent:* Wednesday, June 08, 2016 12:26 PM
> *To:* Robles, Raymond C <raymond.c.robles at intel.com>
> *Cc:* 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> 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> 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]
> *Sent:* Tuesday, June 07, 2016 2:28 PM
> *To:* Robles, Raymond C <raymond.c.robles at intel.com>
> *Cc:* 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> 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] *On Behalf Of *Robles, Raymond C
> *Sent:* Thursday, January 14, 2016 4:55 PM
> *To:* 'nvmewin at lists.openfabrics.org' <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
>
>
>
> *[image: cid:image001.png at 01CB3870.4BB88E70]*
>
> *Raymond C. Robles*
>
> Non-Volatile Memory Solutions Group
>
> Intel Corporation
>
> Office: 480-554-2600
>
> Mobile: 480-399-0645
>
>
>
>
>
>
>
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20160609/a6439948/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 201606092011805_LK7CT9SZ.gif
Type: image/gif
Size: 13168 bytes
Desc: not available
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20160609/a6439948/attachment.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 201606092011780_Z5JE7EUA.png
Type: image/png
Size: 1756 bytes
Desc: not available
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20160609/a6439948/attachment.png>