[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