MINOR: ring: simplify the write loop a little bit

This is mostly a cleanup in that it turns the two-level loop into a
single one, but it also simplifies the code a little bit and brings
some performance savings again, which are mostly noticeable on ARM,
but don't change anything for x86.
This commit is contained in:
Willy Tarreau 2024-03-17 12:09:30 +01:00
parent 573bbbe127
commit 4b984c5baa

View File

@ -265,39 +265,33 @@ ssize_t ring_write(struct ring *ring, size_t maxlen, const struct ist pfx[], siz
* tail to be unlocked again. We stop doing that if another thread * tail to be unlocked again. We stop doing that if another thread
* comes in and becomes the leader in turn. * comes in and becomes the leader in turn.
*/ */
while (1) {
next_cell = &cell;
/* Wait for another thread to take the lead or for the tail to /* Wait for another thread to take the lead or for the tail to
* be available again. It's critical to be read-only in this * be available again. It's critical to be read-only in this
* loop so as not to lose time synchronizing cache lines. Also, * loop so as not to lose time synchronizing cache lines. Also,
* we must detect a new leader ASAP so that the fewest possible * we must detect a new leader ASAP so that the fewest possible
* threads check the tail. * threads check the tail.
*/ */
/* the tail is available again and we're still the leader, try
* again.
*/
while (1) { while (1) {
next_cell = HA_ATOMIC_LOAD(ring_queue_ptr); next_cell = HA_ATOMIC_LOAD(ring_queue_ptr);
if (next_cell != &cell) if (next_cell != &cell)
goto wait_for_flush; // FIXME: another thread arrived, we should go to wait now goto wait_for_flush; // another thread arrived, we should go to wait now
__ha_cpu_relax_for_read(); __ha_cpu_relax_for_read();
#if defined(__x86_64__) #if defined(__x86_64__)
/* x86 prefers a read first */ /* x86 prefers a read first */
if (!(HA_ATOMIC_LOAD(tail_ptr) & RING_TAIL_LOCK)) if (!(HA_ATOMIC_LOAD(tail_ptr) & RING_TAIL_LOCK))
#endif #endif
{ {
tail_ofs = HA_ATOMIC_FETCH_OR(tail_ptr, RING_TAIL_LOCK);
if (!(tail_ofs & RING_TAIL_LOCK))
break;
}
__ha_cpu_relax_for_read();
}
/* OK the queue is locked, let's attempt to get the tail lock */ /* OK the queue is locked, let's attempt to get the tail lock */
tail_ofs = HA_ATOMIC_FETCH_OR(tail_ptr, RING_TAIL_LOCK);
/* did we get it ? */
if (!(tail_ofs & RING_TAIL_LOCK)) { if (!(tail_ofs & RING_TAIL_LOCK)) {
/* Yes! Are we still legitimate to get it ? We'll know by /* We got it! Are we still legitimate to get it ?
* trying to reset the head and replace it with ourselves. * We'll know by trying to reset the head and
* replace it with ourselves.
*/ */
curr_cell = &cell; curr_cell = &cell;
if (!HA_ATOMIC_CAS(ring_queue_ptr, &curr_cell, NULL)) { if (!HA_ATOMIC_CAS(ring_queue_ptr, &curr_cell, NULL)) {
@ -309,6 +303,8 @@ ssize_t ring_write(struct ring *ring, size_t maxlen, const struct ist pfx[], siz
break; break;
} }
} }
__ha_cpu_relax_for_read();
}
head_ofs = HA_ATOMIC_LOAD(&ring->storage->head); head_ofs = HA_ATOMIC_LOAD(&ring->storage->head);