[nvmewin] nvmewin Digest, Vol 4, Issue 27

Harris, James R james.r.harris at intel.com
Thu Apr 19 08:35:21 PDT 2012


This code looks good.  I think you can do one further simplification:

Instead of:

	if (modulo || unaligned) {
		nextPage = physicalAddress.LowPart + PAGE_SIZE;
		nextPage = nextPage & ~PAGE_MASK;
		numImplicitEntries +=
			((physicalAddress.LowPart + modulo) > nextPage) ? 2 : 1;
	}

How about:

	if (modulo || unaligned) {
		numImplicitEntries += 1 + (modulo + unaligned - 1) / PAGE_SIZE;
	}

I'd work out all the examples myself, but since you have your nifty unit test program, maybe you can test it for me.  :-)

Thanks,

-Jim


>-----Original Message-----
>From: Luse, Paul E
>Sent: Thursday, April 19, 2012 6:34 AM
>To: Harris, James R; 'nvmewin at lists.openfabrics.org'
>Subject: RE: nvmewin Digest, Vol 4, Issue 27
>
>OK, here's my user mode test code w/test cases and both current version of the section of
>translateSgl that determines the number of entries and the new one I just came up with.  Both
>pass the unit test cases and I'm currently testing the driver with the new one implemented
>(prev version passed overnight testing).
>
>Please review both the test cases, unit test code and the new algorithm.  I'm proposing using
>the new, of course, provided that it passes all tests and reviews go OK.  There are several
>test cases that are n/a (I didn't include a full test matrix, only the ones that are
>applicable).
>
>Thx
>Paul
>
>PS:  new code actually has 5 operations, had to add one since my last email :)
>
>-----Original Message-----
>From: Luse, Paul E
>Sent: Thursday, April 19, 2012 5:17 AM
>To: Harris, James R; 'nvmewin at lists.openfabrics.org'
>Subject: RE: nvmewin Digest, Vol 4, Issue 27
>
>Actually I've got another more academic approach that I'm testing now and have written unit
>test code to cover all test cases that runs just against this little section that determines
>the number of PPR entries.  Once I have it, and the test code, ready I'll send them out.  We
>should actually have a contest for the simplest/tightest algorithm to solve this fairly simple
>problem... my current routine only has 2 conditions and 5 operations as compared to the 4
>conditions and 11 operations that we currently use
>
>-----Original Message-----
>From: Luse, Paul E
>Sent: Wednesday, April 18, 2012 6:26 PM
>To: Harris, James R; nvmewin at lists.openfabrics.org
>Subject: RE: nvmewin Digest, Vol 4, Issue 27
>
>Actually it doesn't, I don't think.  If you're less than a page size and have an offset and
>just go down the else side you'll end up with a bogus start/end address and the math to
>calculate the num of whole pages will be negative.  I could have generically determined if the
>element crossed a page boundary and if not just set the num entries to 1 and if so drop to the
>else and that would work too.  It wouldn't save any cycles though but would pull the part out
>about calc'ing the next starting page into common code.  I didn't think it was worth it.
>
>Thoughts?
>
>-----Original Message-----
>From: nvmewin-bounces at lists.openfabrics.org [mailto:nvmewin-bounces at lists.openfabrics.org] On
>Behalf Of Harris, James R
>Sent: Wednesday, April 18, 2012 5:47 PM
>To: nvmewin at lists.openfabrics.org
>Subject: Re: [nvmewin] nvmewin Digest, Vol 4, Issue 27
>
>You shouldn't need the "if" part here.  The "else" part will handle the "less than PAGE_SIZE"
>case.
>
>-Jim
>
>
>>-----Original Message-----
>>From: nvmewin-bounces at lists.openfabrics.org
>>[mailto:nvmewin-bounces at lists.openfabrics.org] On Behalf Of
>>nvmewin-request at lists.openfabrics.org
>>Sent: Wednesday, April 18, 2012 5:55 PM
>>To: nvmewin at lists.openfabrics.org
>>Subject: nvmewin Digest, Vol 4, Issue 27
>>
>>Send nvmewin mailing list submissions to
>>	nvmewin at lists.openfabrics.org
>>
>>To subscribe or unsubscribe via the World Wide Web, visit
>>	http://lists.openfabrics.org/cgi-bin/mailman/listinfo/nvmewin
>>or, via email, send a message with subject or body 'help' to
>>	nvmewin-request at lists.openfabrics.org
>>
>>You can reach the person managing the list at
>>	nvmewin-owner at lists.openfabrics.org
>>
>>When replying, please edit your Subject line so it is more specific
>>than "Re: Contents of nvmewin digest..."
>>
>>
>>Today's Topics:
>>
>>   1. updated SGL conversion routine (Luse, Paul E)
>>
>>
>>----------------------------------------------------------------------
>>
>>Message: 1
>>Date: Thu, 19 Apr 2012 00:43:02 +0000
>>From: "Luse, Paul E" <paul.e.luse at intel.com>
>>To: "nvmewin at lists.openfabrics.org" <nvmewin at lists.openfabrics.org>
>>Subject: [nvmewin] updated SGL conversion routine
>>Message-ID:
>>	<82C9F782B054C94B9FC04A331649C77A0BAFC5 at FMSMSX106.amr.corp.intel.com>
>>Content-Type: text/plain; charset="us-ascii"
>>
>>OK, the else has now been updated (thanks Jim, needed some tweaks
>>though but I saw what you were going for of course).  There's so many
>>way to handle this problem, I'm sure we can find something more efficient than this but its
>pretty good I think and easy to follow.
>>
>>Please review, I have this under test now and am heading out....
>>
>>Thx
>>Paul
>>
>>
>>        physicalAddress.QuadPart =
>> pSgl->List[index].PhysicalAddress.QuadPart;
>>
>>        if (sgElementSize <= PAGE_SIZE) {
>>            ULONG nextPage = 0;
>>
>>            /* Must transfer at least one full page */
>>            numImplicitEntries = 1;
>>            /*
>>             * since we may be offset into the page, look to determine
>>             * address to determine if a 2nd PRP entry is needed
>>             */
>>            nextPage = physicalAddress.LowPart + PAGE_SIZE;
>>            nextPage = nextPage & ~PAGE_MASK;
>>            numImplicitEntries +=
>>                ((physicalAddress.LowPart + sgElementSize) > nextPage) ? 1 : 0;
>>        } else {
>>            PHYSICAL_ADDRESS startAddr, endAddr;
>>
>>            startAddr.QuadPart = physicalAddress.QuadPart;
>>            endAddr.QuadPart = startAddr.QuadPart + sgElementSize;
>>            numImplicitEntries = 0;
>>
>>            /* account for an entry becaue of an offset start */
>>            if (startAddr.QuadPart % PAGE_SIZE) {
>>                numImplicitEntries += 1;
>>                /* bump the start address to the next page boundary */
>>                startAddr.QuadPart += PAGE_SIZE;
>>                startAddr.QuadPart &= ~PAGE_MASK;
>>            }
>>
>>            /* account for an entry becaue of tail */
>>            if (endAddr.QuadPart % PAGE_SIZE) {
>>                numImplicitEntries += 1;
>>                /* lop off the tail */
>>                endAddr.QuadPart &= ~PAGE_MASK;
>>            }
>>            /* now account for the whole pages if any */
>>            numImplicitEntries += (ULONG)((endAddr.QuadPart - startAddr.QuadPart) /
>PAGE_SIZE);
>>        }
>>
>>____________________________________
>>Paul Luse
>>Sr. Staff Engineer
>>PCG Server Software Engineering
>>Desk: 480.554.3688, Mobile: 480.334.4630
>>
>>-------------- next part -------------- An HTML attachment was
>>scrubbed...
>>URL:
>><http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120419/29
>>59ab31/attachment.html>
>>-------------- next part -------------- An embedded and
>>charset-unspecified text was scrubbed...
>>Name: nvmeSnti.c
>>URL:
>><http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120419/29
>>59ab31/attachment.c>
>>
>>------------------------------
>>
>>_______________________________________________
>>nvmewin mailing list
>>nvmewin at lists.openfabrics.org
>>http://lists.openfabrics.org/cgi-bin/mailman/listinfo/nvmewin
>>
>>
>>End of nvmewin Digest, Vol 4, Issue 27
>>**************************************
>_______________________________________________
>nvmewin mailing list
>nvmewin at lists.openfabrics.org
>http://lists.openfabrics.org/cgi-bin/mailman/listinfo/nvmewin



More information about the nvmewin mailing list