<br><br>On Saturday, December 8, 2012, Mathieu Desnoyers wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">* Lai Jiangshan (<a href="javascript:;" onclick="_e(event, 'cvml', 'eag0628@gmail.com')">eag0628@gmail.com</a>) wrote:<br>
> we can define rcu_gp_ctr and registry with aligned attribute, but it is not<br>
> reliable way<br>
><br>
> We need only this:<br>
> unsigned long rcu_gp_ctr __attribute((aligned and padded(don't put other<br>
> var next to it except the futex)))<br>
<br>
In which situation would this be unreliable ?</blockquote><div><br></div><br><div>int a;</div><div>int b __attribute__((aligned));</div><div>int c;</div><div><br></div><div>b and c will be in the same line, even we define c as aligned too, the compiler/linker may put a next to b, thus a and b in the same line<span></span></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
<br>
Mathieu<br>
<br>
><br>
> On Saturday, December 8, 2012, Mathieu Desnoyers wrote:<br>
><br>
> > * Lai Jiangshan (<a href="javascript:;" onclick="_e(event, 'cvml', 'laijs@cn.fujitsu.com')">laijs@cn.fujitsu.com</a> <javascript:;>) wrote:<br>
> > > @rcu_gp_ctr and @registry share the same cache line, it causes<br>
> > > false sharing and slowdown both of the read site and update site.<br>
> > ><br>
> > > Fix: Use different cache line for them.<br>
> > ><br>
> > > Although rcu_gp_futex is updated less than rcu_gp_ctr, but<br>
> > > they always be accessed at almost the same time, so we also move<br>
> > rcu_gp_futex<br>
> > > to the cacheline of rcu_gp_ctr to reduce the cacheline-usage or<br>
> > cache-missing<br>
> > > of read site.<br>
> ><br>
> > Hi Lai,<br>
> ><br>
> > I agree on the goal: placing registry and rcu_gp_ctr on different<br>
> > cache-lines. And yes, it makes sense to put rcu_gp_ctr and rcu_gp_futex<br>
> > on the same cache-line. I agree that the next patch is fine too (keeping<br>
> > qsbr and other urcu similar). This is indeed what I try to ensure<br>
> > myself.<br>
> ><br>
> > I'm just concerned that this patch seems to break ABI compability for<br>
> > liburcu: the read-side, within applications, would have to be<br>
> > recompiled. So I guess we should decide if we do this change in a way<br>
> > that does not break the ABI (e.g. not introducing a structure), or if we<br>
> > choose to bump the library version number.<br>
> ><br>
> > Thoughts ?<br>
> ><br>
> > Thanks,<br>
> ><br>
> > Mathieu<br>
> ><br>
> > ><br>
> > ><br>
> > > test: (4X6=24 CPUs)<br>
> > ><br>
> > > Before patch:<br>
> > ><br>
> > > [root@localhost userspace-rcu]# ./tests/test_urcu_mb 20 1 20<br>
> > > SUMMARY ./tests/test_urcu_mb testdur 20 nr_readers 20 rdur<br>
> > 0 wdur 0 nr_writers 1 wdelay 0 nr_reads 2100285330 nr_writes<br>
> > 3390219 nr_ops 2103675549<br>
> > > [root@localhost userspace-rcu]# ./tests/test_urcu_mb 20 1 20<br>
> > > SUMMARY ./tests/test_urcu_mb testdur 20 nr_readers 20 rdur<br>
> > 0 wdur 0 nr_writers 1 wdelay 0 nr_reads 1619868562 nr_writes<br>
> > 3529478 nr_ops 1623398040<br>
> > > [root@localhost userspace-rcu]# ./tests/test_urcu_mb 20 1 20<br>
> > > SUMMARY ./tests/test_urcu_mb testdur 20 nr_readers 20 rdur<br>
> > 0 wdur 0 nr_writers 1 wdelay 0 nr_reads 1949067038 nr_writes<br>
> > 3469334 nr_ops 1952536372<br>
> > ><br>
> > ><br>
> > > after patch:<br>
> > ><br>
> > > [root@localhost userspace-rcu]# ./tests/test_urcu_mb 20 1 20<br>
> > > SUMMARY ./tests/test_urcu_mb testdur 20 nr_readers 20 rdur<br>
> > 0 wdur 0 nr_writers 1 wdelay 0 nr_reads 3380191848 nr_writes<br>
> > 4903248 nr_ops 3385095096<br>
> > > [root@localhost userspace-rcu]# ./tests/test_urcu_mb 20 1 20<br>
> > > SUMMARY ./tests/test_urcu_mb testdur 20 nr_readers 20 rdur<br>
> > 0 wdur 0 nr_writers 1 wdelay 0 nr_reads 3397637486 nr_writes<br>
> > 4129809 nr_ops 3401767295<br>
> > ><br>
> > > Singed-by: Lai Jiangshan <<a href="javascript:;" onclick="_e(event, 'cvml', 'laijs@cn.fujitsu.com')">laijs@cn.fujitsu.com</a> <javascript:;>><br>
> > > ---<br>
> > > diff --git a/urcu.c b/urcu.c<br>
> > > index 15def09..436d71c 100644<br>
> > > --- a/urcu.c<br>
> > > +++ b/urcu.c<br>
> > > @@ -83,16 +83,7 @@ void __attribute__((destructor)) rcu_exit(void);<br>
> > > #endif<br>
> > ><br>
> > > static pthread_mutex_t rcu_gp_lock = PTHREAD_MUTEX_INITIALIZER;<br>
> > > -<br>
> > > -int32_t rcu_gp_futex;<br>
> > > -<br>
> > > -/*<br>
> > > - * Global grace period counter.<br>
> > > - * Contains the current RCU_GP_CTR_PHASE.<br>
> > > - * Also has a RCU_GP_COUNT of 1, to accelerate the reader fast path.<br>
> > > - * Written to only by writer with mutex taken. Read by both writer and<br>
> > readers.<br>
> > > - */<br>
> > > -unsigned long rcu_gp_ctr = RCU_GP_COUNT;<br>
> > > +struct urcu_gp rcu_gp = { .ctr = RCU_GP_COUNT };<br>
> > ><br>
> > > /*<br>
> > > * Written to only by each individual reader. Read by both the reader<br>
> > and the<br>
> > > @@ -217,8 +208,8 @@ static void wait_gp(void)<br>
> > > {<br>
> > > /* Read reader_gp before read futex */<br>
> > > smp_mb_master(RCU_MB_GROUP);<br>
> > > - if (uatomic_read(&rcu_gp_futex) == -1)<br>
> > > - futex_async(&rcu_gp_futex, FUTEX_WAIT, -1,<br>
> > > + if (uatomic_read(&rcu_gp.futex) == -1)<br>
> > > + futex_async(&rcu_gp.futex, FUTEX_WAIT, -1,<br>
> > > NULL, NULL, 0);<br>
> > > }<br>
> > ><br>
> > > @@ -232,12 +223,12 @@ static void wait_for_readers(struct cds_list_head<br>
> > *input_readers,<br>
> > > /*<br>
> > > * Wait for each thread URCU_TLS(rcu_reader).ctr to either<br>
> > > * indicate quiescence (not nested), or observe the current<br>
> > > - * rcu_gp_ctr value.<br>
> > > + * rcu_gp.ctr value.<br>
> > > */<br>
> > > for (;;) {<br>
> > > wait_loops++;<br>
> > > if (wait_loops == RCU_QS_ACTIVE_ATTEMPTS) {<br>
> > > - uatomic_dec(&rcu_gp_futex);<br>
> > > + uatomic_dec(&rcu_gp.futex);<br>
> > > /* Write futex before read reader_gp */<br>
> > > smp_mb_master(RCU_MB_GROUP);<br>
> > > }<br>
> > > @@ -270,7 +261,7 @@ static void wait_for_readers(struct cds_list_head<br>
> > *input_readers,<br>
> > > if (wait_loops == RCU_QS_ACTIVE_ATTEMPTS) {<br>
> > > /* Read reader_gp before write futex */<br>
> > > smp_mb_master(RCU_MB_GROUP);<br>
> > > - uatomic_set(&rcu_gp_futex, 0);<br>
> > > + uatomic_set(&rcu_gp.futex, 0);<br>
> > > }<br>
> > > break;<br>
> > > } else {<br>
> > > @@ -289,7 +280,7 @@ static void wait_for_readers(struct cds_list_head<br>
> > *input_readers,<br>
> > > if (wait_loops == RCU_QS_ACTIVE_ATTEMPTS) {<br>
> > > /* Read reader_gp before write futex */<br>
> > > > > <a href="javascript:;" onclick="_e(event, 'cvml', 'lttng-dev@lists.lttng.org')">lttng-dev@lists.lttng.org</a> <javascript:;><br>
> > <a href="http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev" target="_blank">http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev</a><br>
> ><br>
<br>
--<br>
Mathieu Desnoyers<br>
Operating System Efficiency R&D Consultant<br>
EfficiOS Inc.<br>
<a href="http://www.efficios.com" target="_blank">http://www.efficios.com</a><br>
</blockquote>