<div dir="auto"><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 31, 2022, 7:52 AM Zebediah Figura wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 5/30/22 14:20, Jinoh Kang wrote:<br>
> +<br>
> +    /* GetOverlappedResultEx() skips waiting for the event/file handle if the<br>
> +     * Status field indicates completion, and returns the result from the<br>
> +     * Information field.<br>
> +     *<br>
> +     * Hence, we have to ensure that writes to the Information field are seen<br>
> +     * from the other threads *before* writes to the Status field.<br>
> +     *<br>
> +     * Otherwise, a race condition results: if writing the Status field has<br>
> +     * been completed but the subsequent update to the Information field has<br>
> +     * not, the application may end up reading stale value from it.<br>
> +     *<br>
> +     * The race condition can be especially problematic with applications that<br>
> +     * use the HasOverlappedIoCompleted() macro in a tight loop to busy-wait<br>
> +     * for completion of overlapped I/O.<br>
> +     */<br>
> +    MemoryBarrier();  /* TODO: consider replacing with a store-release. */<br>
> +    *(volatile NTSTATUS *)status_ptr = status;<br>
>   }<br>
>   <br>
>   static inline client_ptr_t iosb_client_ptr( IO_STATUS_BLOCK *io )<br>
<br>
Unpaired MemoryBarrier() is pretty much always wrong. (In fact, I think <br>
unpaired anything would be, here.)<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">By pairing, do you mean that another MemoryBarrier() should follow the status write?</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
It also doesn't seem clear to me that it's actually better than a <br>
store-release, as you point out. Ideally what we want is ReadAcquire() <br>
and WriteRelease(), but even setting that aside, a pair of interlocked <br>
operations will probably be an improvement by themselves.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">As in, convert both status and information writes to interlocked operations, or replace the barrier with a dummy atomic op?</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div></div>