Re: [PATCH v7 0/8] MR1628: ntdll: Implement Low Fragmentation Heap.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntdll/heap.c:
> -
>    if ((group = InterlockedExchangePointer( (void *)&bin->affinity_group[affinity], group )))
> -
>        RtlInterlockedPushEntrySList( &bin->groups, (SLIST_ENTRY *)&group->entry );
> - }
> -
> - return block;
>
> +}
> -
>
> +static NTSTATUS heap_allocate_block_lfh( struct heap *heap, ULONG flags, SIZE_T block_size,
> -
>                                     SIZE_T size, void **ret )
>
> +{
> - struct bin *bin, *last = heap->bins + BLOCK_SIZE_BIN_COUNT - 1;
> - struct block *block;
> -
> - bin = heap->bins + BLOCK_SIZE_BIN( block_size );
> - if (!heap->bins || bin == last) return STATUS_UNSUCCESSFUL;
> - if (!ReadAcquire( &bin->enabled )) return STATUS_UNSUCCESSFUL;

I believe that an atomic operation with acquire/release semantics needs to be accompanied by documentation that explains which corresponding operation with release/acquire semantics it is paired with, at both sides.  See `GetOverlappedResult` in Wine, or the Linux kernel source tree for details.

```suggestion:-0+0

/* paired with WriteRelease in bin_try_enable. */
    if (!ReadAcquire( &bin->enabled )) return STATUS_UNSUCCESSFUL;
```