Zebediah Figura (@zfigura) commented about dlls/ntdll/sync.c:
- if (!old.s.owners) + struct srwlock_waiter *tmp = NULL; + if (last_to_remove != last) + tmp = ReadPointerAcquire((void **)&last_to_remove->next); + if (tmp != new_head) { - /* Not locked exclusive or shared. We can try to grab it. */ - new.s.owners = -1; - --new.s.exclusive_waiters; - wait = FALSE; + new_head = tmp; + last_up_to_date = last_to_remove; } - else + if (!new_head) + break; last_to_remove should always be the entry right before new_head, right? And if last_to_remove == last then new_head == NULL, and vice versa, right? And if new_head is NULL, then we're removing everything from the list and we don't need to update any head pointers.
So unless I'm missing something, we can simplify this part a bit, to: ```c if (last_to_remove == last) break; tmp = ReadAcquire(&last_to_remove->next); if (tmp != new_head) { new_head = tmp; last_up_to_date = last_to_remove; } ``` Somewhat relatedly, does the head pointer actually need to be accurate for every waiter? It's only ever read from the tail. Even if so, I wonder if we can just simplify the "last_up_to_date" tracking in general. After playing with it a bit I came up with the following code structure; maybe this is a bit easier to follow? ```c new_head = NULL; last_up_to_date = last; for (;;) /* cmpxchg loop */ { if (!new_head) { for (;;) { /* walk the list, update last_to_remove */ } last_up_to_date = last_to_remove; } while (last_up_to_date != last) /* update heads */ } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3504#note_43105