* Lai Jiangshan (la...@cn.fujitsu.com) wrote:
> urcu-qsbr's read site's quiescence is much longer than normal urcu ==>
> synchronize_rcu() is much slower ==>
> rcu_gp_ctr is updated much less ==>
> the whole urcu-qsbr will not be slowed down by false sharing of rcu_gp_ctr.
Hi Lai,
Sorry for the late reply, I must have been very busy when you sent this
patch. It's indeed a very good idea to make qsbr more similar to other
URCU flavors. I'm merging it right away.
Thanks!
Mathieu
>
> But this patch makes sense to keep the code of urcu-qsbr like normal urcu,
> better readability and maintenance.
>
> Test: (4*6 CPUs)
> Before patch:
> [root@localhost userspace-rcu]# ./tests/test_urcu_qsbr 20 1 20
> SUMMARY ./tests/test_urcu_qsbr testdur 20 nr_readers 20 rdur 0
> wdur 0 nr_writers 1 wdelay 0 nr_reads 65498297587 nr_writes
> 2000665 nr_ops 65500298252
> [root@localhost userspace-rcu]# ./tests/test_urcu_qsbr 20 1 20
> SUMMARY ./tests/test_urcu_qsbr testdur 20 nr_readers 20 rdur 0
> wdur 0 nr_writers 1 wdelay 0 nr_reads 67218079467 nr_writes
> 1981593 nr_ops 67220061060
>
> After patch
> ./tests/test_urcu_qsbr 20 1 20
> SUMMARY ./tests/test_urcu_qsbr testdur 20 nr_readers 20 rdur 0
> wdur 0 nr_writers 1 wdelay 0 nr_reads 67473798999 nr_writes
> 1999151 nr_ops 67475798150
> [root@localhost userspace-rcu]# ./tests/test_urcu_qsbr 20 1 20
> SUMMARY ./tests/test_urcu_qsbr testdur 20 nr_readers 20 rdur 0
> wdur 0 nr_writers 1 wdelay 0 nr_reads 67065521397 nr_writes
> 1993956 nr_ops 67067515353
>
>
> Signed-off-by: Lai Jiangshan <la...@cn.fujitsu.com>
> --
> diff --git a/urcu-qsbr.c b/urcu-qsbr.c
> index 3c2c65d..995c564 100644
> --- a/urcu-qsbr.c
> +++ b/urcu-qsbr.c
> @@ -53,13 +53,7 @@
> void __attribute__((destructor)) rcu_exit(void);
>
> static pthread_mutex_t rcu_gp_lock = PTHREAD_MUTEX_INITIALIZER;
> -
> -int32_t rcu_gp_futex;
> -
> -/*
> - * Global grace period counter.
> - */
> -unsigned long rcu_gp_ctr = RCU_GP_ONLINE;
> +struct urcu_gp rcu_gp = { .ctr = RCU_GP_ONLINE };
>
> /*
> * Active attempts to check for reader Q.S. before calling futex().
> @@ -118,8 +112,8 @@ static void wait_gp(void)
> {
> /* Read reader_gp before read futex */
> cmm_smp_rmb();
> - if (uatomic_read(&rcu_gp_futex) == -1)
> - futex_noasync(&rcu_gp_futex, FUTEX_WAIT, -1,
> + if (uatomic_read(&rcu_gp.futex) == -1)
> + futex_noasync(&rcu_gp.futex, FUTEX_WAIT, -1,
> NULL, NULL, 0);
> }
>
> @@ -133,12 +127,12 @@ static void wait_for_readers(struct cds_list_head
> *input_readers,
> /*
> * Wait for each thread URCU_TLS(rcu_reader).ctr to either
> * indicate quiescence (offline), or for them to observe the
> - * current rcu_gp_ctr value.
> + * current rcu_gp.ctr value.
> */
> for (;;) {
> wait_loops++;
> if (wait_loops >= RCU_QS_ACTIVE_ATTEMPTS) {
> - uatomic_set(&rcu_gp_futex, -1);
> + uatomic_set(&rcu_gp.futex, -1);
> /*
> * Write futex before write waiting (the other side
> * reads them in the opposite order).
> @@ -177,7 +171,7 @@ static void wait_for_readers(struct cds_list_head
> *input_readers,
> if (wait_loops >= RCU_QS_ACTIVE_ATTEMPTS) {
> /* Read reader_gp before write futex */
> cmm_smp_mb();
> - uatomic_set(&rcu_gp_futex, 0);
> + uatomic_set(&rcu_gp.futex, 0);
> }
> break;
> } else {
> @@ -253,11 +247,11 @@ void synchronize_rcu(void)
>
> /*
> * Must finish waiting for quiescent state for original parity
> - * before committing next rcu_gp_ctr update to memory. Failure
> + * before committing next rcu_gp.ctr update to memory. Failure
> * to do so could result in the writer waiting forever while new
> * readers are always accessing data (no progress). Enforce
> * compiler-order of load URCU_TLS(rcu_reader).ctr before store
> - * to rcu_gp_ctr.
> + * to rcu_gp.ctr.
> */
> cmm_barrier();
>
> @@ -269,13 +263,13 @@ void synchronize_rcu(void)
> cmm_smp_mb();
>
> /* Switch parity: 0 -> 1, 1 -> 0 */
> - CMM_STORE_SHARED(rcu_gp_ctr, rcu_gp_ctr ^ RCU_GP_CTR);
> + CMM_STORE_SHARED(rcu_gp.ctr, rcu_gp.ctr ^ RCU_GP_CTR);
>
> /*
> - * Must commit rcu_gp_ctr update to memory before waiting for
> + * Must commit rcu_gp.ctr update to memory before waiting for
> * quiescent state. Failure to do so could result in the writer
> * waiting forever while new readers are always accessing data
> - * (no progress). Enforce compiler-order of store to rcu_gp_ctr
> + * (no progress). Enforce compiler-order of store to rcu_gp.ctr
> * before load URCU_TLS(rcu_reader).ctr.
> */
> cmm_barrier();
> @@ -353,13 +347,13 @@ void synchronize_rcu(void)
> goto out;
>
> /* Increment current G.P. */
> - CMM_STORE_SHARED(rcu_gp_ctr, rcu_gp_ctr + RCU_GP_CTR);
> + CMM_STORE_SHARED(rcu_gp.ctr, rcu_gp.ctr + RCU_GP_CTR);
>
> /*
> - * Must commit rcu_gp_ctr update to memory before waiting for
> + * Must commit rcu_gp.ctr update to memory before waiting for
> * quiescent state. Failure to do so could result in the writer
> * waiting forever while new readers are always accessing data
> - * (no progress). Enforce compiler-order of store to rcu_gp_ctr
> + * (no progress). Enforce compiler-order of store to rcu_gp.ctr
> * before load URCU_TLS(rcu_reader).ctr.
> */
> cmm_barrier();
> diff --git a/urcu/static/urcu-qsbr.h b/urcu/static/urcu-qsbr.h
> index f6e5580..2b9bece 100644
> --- a/urcu/static/urcu-qsbr.h
> +++ b/urcu/static/urcu-qsbr.h
> @@ -119,12 +119,18 @@ static inline void rcu_debug_yield_init(void)
> #define RCU_GP_ONLINE (1UL << 0)
> #define RCU_GP_CTR (1UL << 1)
>
> -/*
> - * Global quiescent period counter with low-order bits unused.
> - * Using a int rather than a char to eliminate false register dependencies
> - * causing stalls on some architectures.
> - */
> -extern unsigned long rcu_gp_ctr;
> +struct urcu_gp {
> + /*
> + * Global quiescent period counter with low-order bits unused.
> + * Using a int rather than a char to eliminate false register
> + * dependencies causing stalls on some architectures.
> + */
> + unsigned long ctr;
> +
> + int32_t futex;
> +} __attribute__((aligned(CAA_CACHE_LINE_SIZE)));
> +
> +extern struct urcu_gp rcu_gp;
>
> struct rcu_reader {
> /* Data used by both reader and synchronize_rcu() */
> @@ -137,8 +143,6 @@ struct rcu_reader {
>
> extern DECLARE_URCU_TLS(struct rcu_reader, rcu_reader);
>
> -extern int32_t rcu_gp_futex;
> -
> /*
> * Wake-up waiting synchronize_rcu(). Called from many concurrent threads.
> */
> @@ -147,10 +151,10 @@ static inline void wake_up_gp(void)
> if (caa_unlikely(_CMM_LOAD_SHARED(URCU_TLS(rcu_reader).waiting))) {
> _CMM_STORE_SHARED(URCU_TLS(rcu_reader).waiting, 0);
> cmm_smp_mb();
> - if (uatomic_read(&rcu_gp_futex) != -1)
> + if (uatomic_read(&rcu_gp.futex) != -1)
> return;
> - uatomic_set(&rcu_gp_futex, 0);
> - futex_noasync(&rcu_gp_futex, FUTEX_WAKE, 1,
> + uatomic_set(&rcu_gp.futex, 0);
> + futex_noasync(&rcu_gp.futex, FUTEX_WAKE, 1,
> NULL, NULL, 0);
> }
> }
> @@ -162,7 +166,7 @@ static inline enum rcu_state rcu_reader_state(unsigned
> long *ctr)
> v = CMM_LOAD_SHARED(*ctr);
> if (!v)
> return RCU_READER_INACTIVE;
> - if (v == rcu_gp_ctr)
> + if (v == rcu_gp.ctr)
> return RCU_READER_ACTIVE_CURRENT;
> return RCU_READER_ACTIVE_OLD;
> }
> @@ -216,7 +220,7 @@ static inline void
> _rcu_quiescent_state_update_and_wakeup(unsigned long gp_ctr)
> * to be invoked directly from non-LGPL code.
> *
> * We skip the memory barriers and gp store if our local ctr already
> - * matches the global rcu_gp_ctr value: this is OK because a prior
> + * matches the global rcu_gp.ctr value: this is OK because a prior
> * _rcu_quiescent_state() or _rcu_thread_online() already updated it
> * within our thread, so we have no quiescent state to report.
> */
> @@ -224,7 +228,7 @@ static inline void _rcu_quiescent_state(void)
> {
> unsigned long gp_ctr;
>
> - if ((gp_ctr = CMM_LOAD_SHARED(rcu_gp_ctr)) == URCU_TLS(rcu_reader).ctr)
> + if ((gp_ctr = CMM_LOAD_SHARED(rcu_gp.ctr)) == URCU_TLS(rcu_reader).ctr)
> return;
> _rcu_quiescent_state_update_and_wakeup(gp_ctr);
> }
> @@ -257,7 +261,7 @@ static inline void _rcu_thread_offline(void)
> static inline void _rcu_thread_online(void)
> {
> cmm_barrier(); /* Ensure the compiler does not reorder us with mutex */
> - _CMM_STORE_SHARED(URCU_TLS(rcu_reader).ctr,
> CMM_LOAD_SHARED(rcu_gp_ctr));
> + _CMM_STORE_SHARED(URCU_TLS(rcu_reader).ctr,
> CMM_LOAD_SHARED(rcu_gp.ctr));
> cmm_smp_mb();
> }
>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
