[nvmewin] Bug Fix Patch - Review Request

[Luse, Paul E]

Well that's odd.  Let me see if I can reproduce it here and if so ill ping the storport guys and if not we can compare configs

Sent from my iPhone

On Nov 5, 2012, at 11:52 AM, "Chang, Alex" <Alex.Chang at idt.com<mailto:Alex.Chang at idt.com>> wrote:

Hi Paul,

Last Friday, when I ran IOMeter with 4K random reads with latest patch, the performance numbers decreases by half, e.g. number of IOs. After debugging it, the reason is StorPortGetUncachedExtension calling. I am quite sure why. However, if the assertion only happen on check-built Windows 8, I think we should add the calling for that specific case only. I wonder you guys see the performance drop ever?

Thanks,
Alex

________________________________
From: Luse, Paul E [mailto:paul.e.luse at intel.com]
Sent: Thursday, October 25, 2012 10:53 AM
To: Chang, Alex; Murray, Kris R; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: RE: Bug Fix Patch - Review Request

I think the answer is that we don’t care.  Msft suggested that we avoid causing assertions in the checked OS; that’s all this does.  We don’t use the mem (and don’t have to free it) so the only issue that would be present if it failed would be if someone happened to be running on a checked OS when that happened, they’d get an assert in storport just after find adapter and then they could ignore it and move on w/o any further problems….

From: nvmewin-bounces at lists.openfabrics.org<mailto:nvmewin-bounces at lists.openfabrics.org> [mailto:nvmewin-bounces at lists.openfabrics.org] On Behalf Of Chang, Alex
Sent: Thursday, October 25, 2012 10:32 AM
To: Murray, Kris R; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: Re: [nvmewin] Bug Fix Patch - Review Request

My concern is what if the allocation fails by any chance?

Alex

________________________________
From: Murray, Kris R [mailto:kris.r.murray at intel.com]<mailto:[mailto:kris.r.murray at intel.com]>
Sent: Thursday, October 25, 2012 10:23 AM
To: Chang, Alex; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: RE: Bug Fix Patch - Review Request
Alex,
Since we don’t use the returned pointer I believe there is no need to validate it. The goal is to cause Storport to allocate a DMA adapter object. See the attached email from James Harris for more info.
~Kris

From: Chang, Alex [mailto:Alex.Chang at idt.com]<mailto:[mailto:Alex.Chang at idt.com]>
Sent: Thursday, October 25, 2012 10:11 AM
To: Murray, Kris R; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: RE: Bug Fix Patch - Review Request

Hi Kris,

I have a quick question regarding StorPortGetUncachedExtension:
The routine returns a pointer to the allocated buffer, should we validate the pointer before proceeding?

Thanks,
Alex

________________________________
From: nvmewin-bounces at lists.openfabrics.org<mailto:nvmewin-bounces at lists.openfabrics.org> [mailto:nvmewin-bounces at lists.openfabrics.org]<mailto:[mailto:nvmewin-bounces at lists.openfabrics.org]> On Behalf Of Murray, Kris R
Sent: Tuesday, October 16, 2012 10:07 AM
To: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: [nvmewin] Bug Fix Patch - Review Request
Hi all,

The attached NVMe.zip file changes include the below fixes:

•        nvmeStd.c

o   Added a call to StorPortGetUncachedExtension to fix a checked OS assertion

•        nvmeSnti.c

o   Fixed SntiTranslateRead6 function to use the Read mask for the lba instead of the write mask

o   Fixed SntiTranslateWrite6 function to use the correct macro for getting 24 bits from the CDB using the correct offset

•        nvmeSntiTypes.h

o   Updated READ_6_CDB_LBA_MASK definition to match the one for write

o   Fixed WRITE_6_CDB_LBA_OFFSET from 0 to 1

The attached Results.zip file contains results from the test matrix below:

Operating Systems:

•        Windows 7

•        Windows 8

•        Windows Server 2008

•        Windows Server 2012

Tests:

•        IOMeter

•        SCSI Compliance

•        PCMark

Please review the changes, feeling free to send me comments and questions.

Thanks,
~Kris Murray