[tkreuzer] 56783: [RTL/AMD64] - Fix bugs in RtlInterlockedFlushSList - Improve comments

Author: tkreuzer
Date: Fri Jun 22 10:26:00 2012
New Revision: 56783
URL: http://svn.reactos.org/svn/reactos?rev=56783&view=rev
Log:
[RTL/AMD64]
- Fix bugs in RtlInterlockedFlushSList
- Improve comments
Modified:
    trunk/reactos/lib/rtl/amd64/slist.S
Modified: trunk/reactos/lib/rtl/amd64/slist.S
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/amd64/slist.S?rev=…
==============================================================================
--- trunk/reactos/lib/rtl/amd64/slist.S [iso-8859-1] (original)
+++ trunk/reactos/lib/rtl/amd64/slist.S [iso-8859-1] Fri Jun 22 10:26:00 2012
@@ -8,6 +8,44 @@
 #include <asm.inc>
 #include <ksamd64.inc>
+
+/*
+typedef union DECLSPEC_ALIGN(16) _SLIST_HEADER
+{
+    struct
+    {
+        ULONGLONG Alignment;
+        ULONGLONG Region;
+    };
+    struct
+    {
+        ULONGLONG Depth:16;
+        ULONGLONG Sequence:9;
+        ULONGLONG NextEntry:39;
+        ULONGLONG HeaderType:1;
+        ULONGLONG Init:1;
+        ULONGLONG Reserved:59;
+        ULONGLONG Region:3;
+    } Header8;
+    struct
+    {
+        ULONGLONG Depth:16;
+        ULONGLONG Sequence:48;
+        ULONGLONG HeaderType:1;
+        ULONGLONG Init:1;
+        ULONGLONG Reserved:2;
+        ULONGLONG NextEntry:60;
+    } Header16;
+    struct
+    {
+        ULONGLONG Depth:16;
+        ULONGLONG Sequence:48;
+        ULONGLONG HeaderType:1;
+        ULONGLONG Reserved:3;
+        ULONGLONG NextEntry:60;
+    } HeaderX64;
+} SLIST_HEADER, *PSLIST_HEADER;
+*/
 #define SLIST8A_DEPTH_MASK       HEX(000000000000FFFF)
 #define SLIST8A_DEPTH_INC        HEX(0000000000000001)
@@ -26,7 +64,7 @@
 #define SLIST16A_SEQUENCE_INC    HEX(0000000000010000)
 #define SLIST16B_HEADERTYPE_MASK HEX(0000000000000001)
 #define SLIST16B_INIT_MASK       HEX(0000000000000002)
-#define SLIST16B_NEXTENTY_MASK   HEX(FFFFFFFFFFFFFFF0)
+#define SLIST16B_NEXTENTRY_MASK  HEX(FFFFFFFFFFFFFFF0)
 /* FUNCTIONS ****************************************************************/
@@ -105,7 +143,7 @@
     lock cmpxchg [rcx], r8
     /* If not equal, retry with rax, being the content of [rcx] now */
-    jnz ExpInterlockedPopEntrySListResume
+    jne ExpInterlockedPopEntrySListResume
     /* Shift the pointer bits in place */
     and rax, SLIST8A_NEXTENTRY_MASK
@@ -124,7 +162,10 @@
     ret
 RtlInterlockedPopEntrySList16:
-    /* This is a 16 byte header */
+    /* This is a 16 byte header
+       rcx == ListHead
+       rdx == ListHead->Region
+       rax == ListHead->Alignment */
     /* Save rbx */
     push rbx
@@ -134,18 +175,18 @@
 ExpInterlockedPopEntrySListResume16:
-    /* Check if ListHead->NextEntry is NULL */
+    /* Set r9 = ListHead->NextEntry and check if it is NULL */
     mov r9, rdx
-    and r9, SLIST16B_NEXTENTY_MASK
+    and r9, SLIST16B_NEXTENTRY_MASK
     jz RtlInterlockedPopEntrySListEmpty16
 ExpInterlockedPopEntrySListFault16:
-    /* Get next pointer */
+    /* Set NewListHead.Next = ListHead->NextEntry->Next */
     mov rcx, [r9]
-    /* Set ListHead->HeaderType = 1 and  ListHead->Init = 1 */
-    or rcx, 3
+    /* Set NewListHead.HeaderType = 1 and  NewListHead.Init = 1 */
+    or rcx, (SLIST16B_HEADERTYPE_MASK or SLIST16B_INIT_MASK)
     /* Copy Depth and Sequence number and adjust Depth */
     lea rbx, [rax - SLIST16A_DEPTH_INC]
@@ -156,11 +197,11 @@
     lock cmpxchg16b [r8]
     /* If not equal, retry with rdx:rax, being the content of [r8] now */
-    jnz ExpInterlockedPopEntrySListResume16
+    jne ExpInterlockedPopEntrySListResume16
     /* Copy the old NextEntry pointer to rax */
     mov rax, rdx
-    and rax, SLIST16B_NEXTENTY_MASK
+    and rax, SLIST16B_NEXTENTRY_MASK
     /* Return */
     pop rbx
@@ -193,7 +234,7 @@
     /* Load ListHead->Alignment into rax */
     mov rax, [rcx]
-    /* Load ListHead->Region into rdx */
+    /* Load ListHead->Region into r9 */
     mov r9, [rcx + 8]
     /* Check what kind of header this is */
@@ -242,14 +283,18 @@
     lock cmpxchg [rcx], r8
     /* If not equal, retry with rax, being the content of [rcx] now */
-    jnz RtlInterlockedPushEntrySListLoop
+    jne RtlInterlockedPushEntrySListLoop
     /* Return the old NextEntry pointer */
     mov rax, r9
     ret
 RtlInterlockedPushEntrySList16:
-    /* This is a 16 byte header */
+    /* This is a 16 byte header
+       rcx = ListHead
+       rdx = ListEntry
+       rax = ListHead->Alignment
+       r9 = ListHead->Region */
     /* Save rbx */
     push rbx
@@ -258,33 +303,41 @@
     mov r8, rcx
     mov r9, rdx
-    /* Set ListHead->HeaderType = 1 and  ListHead->Init = 1 */
+    /* Set NewListHead.NextEntry = ListEntry */
     mov rcx, rdx
-    or rcx, 3
-
+
+    /* Set NewListHead.HeaderType = 1 and NewListHead.Init = 1 */
+    or rcx, (SLIST16B_HEADERTYPE_MASK or SLIST16B_INIT_MASK)
+
+    /* Set rdx = ListHead->Region */
     mov rdx, [r8 + 8]
 RtlInterlockedPushEntrySListLoop16:
+    /* r8 = ListHead
+       r9 = ListEntry
+       rax = ListHead->Alignment
+       rdx = ListHead->Region
+    */
     /* Move ListHead->NextEntry to rbx */
     mov rbx, rdx
-    and rbx, SLIST16B_NEXTENTY_MASK
-
-    /* Store next pointer in ListEntry->NextEntry */
+    and rbx, SLIST16B_NEXTENTRY_MASK
+
+    /* Store next pointer in ListEntry->Next */
     mov [r9], rbx
-    /* Copy Depth and Sequence number and adjust Depth */
+    /* Copy and increment Depth and Sequence number to rbx */
     lea rbx, [rax + SLIST16A_DEPTH_INC + SLIST16A_SEQUENCE_INC]
     /* If [r8] equals rdx:rax, exchange it with rcx:rbx */
     lock cmpxchg16b [r8]
     /* If not equal, retry with rdx:rax, being the content of [r8] now */
-    jnz RtlInterlockedPushEntrySListLoop16
+    jne RtlInterlockedPushEntrySListLoop16
     /* Copy the old NextEntry pointer to rax */
     mov rax, rdx
-    and rax, SLIST16B_NEXTENTY_MASK
+    and rax, SLIST16B_NEXTENTRY_MASK
     /* Return */
     pop rbx
@@ -294,46 +347,62 @@
 /* PSLIST_ENTRY
  * NTAPI
  * RtlInterlockedFlushSList(
- *     IN PSINGLE_LIST_ENTRY ListHead);
+ *     IN PSLIST_HEADER ListHead);
  */
 RtlInterlockedFlushSList:
 ExpInterlockedFlushSList:
     /* Load ListHead->Region into rdx */
-    mov rax, [rcx + 8]
+    mov rdx, [rcx + 8]
+
+    /* Load ListHead->Alignment into rax */
+    mov rax, [rcx]
     /* Check what kind of header this is */
-    test rax, SLIST8B_HEADERTYPE_MASK
+    test rdx, SLIST8B_HEADERTYPE_MASK
     jnz RtlInterlockedFlushSList16
     /* We have an 8 byte header */
 RtlInterlockedFlushSListLoop:
-    /* Zero ListHead->Alignment */
+    /* Zero NewListHead.Alignment */
     xor r8, r8
     /* If [rcx] equals rax, exchange it with r8 */
     lock cmpxchg [rcx], r8
     /* If not equal, retry with rax, being the content of [rcx] now */
-    jnz RtlInterlockedFlushSListLoop
-
-    /* Use rcx as pointer template */
-    mov rdx, (not SLIST8_POINTER_MASK)
-    or rdx, rcx
+    jne RtlInterlockedFlushSListLoop
+
+    /* Create a pointer template from rcx in rdx */
+    mov rdx, (NOT SLIST8_POINTER_MASK)
+    and rdx, rcx
+
+    /* Load the old NextEntry pointer into rax */
+    and rax, SLIST8A_NEXTENTRY_MASK
+    shr rax, SLIST8A_NEXTENTRY_SHIFT
     /* Combine result and return */
     or rax, rdx
     ret
 RtlInterlockedFlushSList16:
-    /* We have a 16 byte header */
+    /* We have a 16 byte header
+        rcx = ListHead
+        rax = ListHead->Alignment
+        rdx = ListHead->Region
+    */
+
+    /* Save rbx */
     push rbx
-    mov rdx, [rcx + 8]
+    /* Load ListHead into r8, as we need rcx for the exchange */
+    mov r8, rcx
+
+    /* Initialize an ampty NewListHead in rcx:rbx */
     xor rbx, rbx
-    mov rcx, 3
+    mov rcx, (SLIST16B_HEADERTYPE_MASK or SLIST16B_INIT_MASK)
 RtlInterlockedFlushSListLoop16:
@@ -341,14 +410,15 @@
     lock cmpxchg16b [r8]
     /* If not equal, retry with rdx:rax, being the content of [r8] now */
-    jnz RtlInterlockedFlushSListLoop16
+    jne RtlInterlockedFlushSListLoop16
     /* Copy the old NextEntry pointer to rax */
     mov rax, rdx
-    and rax, SLIST16B_NEXTENTY_MASK
+    and rax, SLIST16B_NEXTENTRY_MASK
     /* Return */
     pop rbx
     ret
 END
+