Skip to content

Commit 697f8f8

Browse files
committed
thread-rcu: Fix incorrect grace period counting
Currently, the reader_func is counting the number of grace periods based on the value of dut, which is updated by rcu_assign_pointer. However, the value of dut actually represents the time we update the value, not the number of grace periods. Also, the original method might result in incorrect counting if someone tried to update the gp_idx while others who saw the same dut value with prev_count still depend on the old gp_idx to increase the counter. To fix the problem, instead of relying on the dut value to increase the gp_idx, we manually increase gp_idx on write side. Then, we can easily determine the gp on read side. For dut value, we simply check the old count value is not greater than the newest one. Additionally, since synchronize_rcu is quite slow, readers generally will pass through the critical section during the first grace period. To generate more realistic output, we add a delay on read side before entering the critical section. Before: 100 reader(s), 5 update run(s), 6 grace period(s) [grace period #0] 100 reader(s) [grace period #1] 0 reader(s) [grace period #2] 0 reader(s) [grace period #3] 0 reader(s) [grace period #4] 0 reader(s) [grace period #5] 0 reader(s) After, we added a delay: 100 reader(s), 5 update run(s), 6 grace period(s) [grace period #0] 76 reader(s) [grace period #1] 0 reader(s) [grace period #2] 1 reader(s) [grace period #3] 0 reader(s) [grace period #4] 3 reader(s) [grace period #5] 20 reader(s)
1 parent fd73065 commit 697f8f8

File tree

3 files changed

+29
-19
lines changed

3 files changed

+29
-19
lines changed

thread-rcu/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
CFLAGS = -Wall
22
CFLAGS += -g
3-
CFLAGS += -std=c11
3+
CFLAGS += -std=gnu11
44
CFLAGS += -D'N_READERS=100'
55
CFLAGS += -D'N_UPDATE_RUN=5'
66
CFLAGS += -fsanitize=thread

thread-rcu/main.c

+27-17
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ static inline void thread_barrier(struct barrier_struct *b, size_t n)
3737
#include <stdint.h>
3838
#include <stdio.h>
3939
#include <stdlib.h>
40+
#include <time.h>
4041

4142
#include "rcu.h"
4243

@@ -55,34 +56,42 @@ static atomic_uint grace_periods[GP_IDX_MAX];
5556
static void *reader_func(void *argv)
5657
{
5758
struct test *tmp;
59+
unsigned int local_gp_idx;
5860
unsigned int old_prev_count;
61+
struct timespec ts = {
62+
.tv_sec = 0,
63+
.tv_nsec = 30000L,
64+
};
5965

6066
if (rcu_init())
6167
abort();
6268

6369
thread_barrier(&test_barrier, N_READERS + 1);
70+
nanosleep(&ts, NULL);
6471

6572
rcu_read_lock();
6673

6774
tmp = rcu_dereference(dut);
6875

69-
if (tmp->count != atomic_load_explicit(&prev_count, memory_order_acquire)) {
70-
old_prev_count = atomic_exchange_explicit(&prev_count, tmp->count,
71-
memory_order_release);
72-
if (tmp->count != old_prev_count)
73-
atomic_fetch_add_explicit(&gp_idx, 1, memory_order_release);
74-
if (atomic_load_explicit(&gp_idx, memory_order_acquire) >
75-
N_UPDATE_RUN) {
76-
fprintf(stderr, "grace period index (%u) is over bound (%u).\n",
77-
atomic_load_explicit(&gp_idx, memory_order_acquire),
78-
N_UPDATE_RUN);
79-
abort();
80-
}
76+
old_prev_count = atomic_load_explicit(&prev_count, memory_order_acquire);
77+
if (old_prev_count < tmp->count) {
78+
atomic_compare_exchange_strong(&prev_count, &old_prev_count,
79+
tmp->count);
80+
} else if (tmp->count < old_prev_count) {
81+
fprintf(stderr,
82+
"old count (%u) should not be larger than new one (%u).\n",
83+
old_prev_count, tmp->count);
84+
abort();
8185
}
8286

83-
atomic_fetch_add_explicit(
84-
&grace_periods[atomic_load_explicit(&gp_idx, memory_order_acquire)], 1,
85-
memory_order_relaxed);
87+
local_gp_idx = atomic_load_explicit(&gp_idx, memory_order_acquire);
88+
if (local_gp_idx > N_UPDATE_RUN) {
89+
fprintf(stderr, "grace period index (%u) is over bound (%u).\n",
90+
local_gp_idx, N_UPDATE_RUN);
91+
abort();
92+
}
93+
atomic_fetch_add_explicit(&grace_periods[local_gp_idx], 1,
94+
memory_order_relaxed);
8695

8796
rcu_read_unlock();
8897

@@ -103,6 +112,7 @@ static void *updater_func(void *argv)
103112
newval->count = i;
104113
oldp = rcu_assign_pointer(dut, newval);
105114
synchronize_rcu();
115+
atomic_fetch_add_explicit(&gp_idx, 1, memory_order_release);
106116
free(oldp);
107117
}
108118

@@ -140,8 +150,8 @@ int main(int argc, char *argv[])
140150

141151
if (total != N_READERS)
142152
fprintf(stderr,
143-
"The Sum of records in the array of grace period(s) (%u) is "
144-
"not the same with number of reader(s) (%u)\n",
153+
"The sum of records in the array of grace period(s) (%u)\n"
154+
"is not the same with number of reader(s) (%u)\n",
145155
total, N_READERS);
146156

147157
return 0;

thread-rcu/rcu.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ struct rcu_data {
168168
} while (0)
169169
#define rcu_next(np) \
170170
((struct rcu_node *) (READ_ONCE((np)->__next_rcu_nesting) & ~0x3))
171-
#define rcu_next_mask(nrn) ((struct rcu_node *) ((uintptr_t)(nrn) & ~0x3))
171+
#define rcu_next_mask(nrn) ((struct rcu_node *) ((uintptr_t) (nrn) & ~0x3))
172172

173173
static struct rcu_data rcu_data = {
174174
.nr_thread = 0,

0 commit comments

Comments
 (0)