[lttng-dev] [PATCH] QSBR: Use xor operation to replace add operation when changing rcu_gp.ctr value

[lttng-dev] [PATCH] QSBR: Use xor operation to replace add operation when changing rcu_gp.ctr value

[yaowenbin via lttng-dev]

It is enough to have three values of rcu_gp.ctr, 00 for INACTIVE,
01 or 11 for ACTIVE. So it is possible to replace add operation
with xor operation when changing rcu_gp.ctr value.

Signed-off-by: yaowenbin <yaowenbin1@huawei.com>
---
 src/urcu-qsbr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/urcu-qsbr.c b/src/urcu-qsbr.c
index 3709412..46135f9 100644
--- a/src/urcu-qsbr.c
+++ b/src/urcu-qsbr.c
@@ -391,7 +391,7 @@ void urcu_qsbr_synchronize_rcu(void)
 		goto out;

 	/* Increment current G.P. */
-	CMM_STORE_SHARED(urcu_qsbr_gp.ctr, urcu_qsbr_gp.ctr + URCU_QSBR_GP_CTR);
+	CMM_STORE_SHARED(urcu_qsbr_gp.ctr, urcu_qsbr_gp.ctr ^ URCU_QSBR_GP_CTR);

 	/*
 	 * Must commit urcu_qsbr_gp.ctr update to memory before waiting for
-- 
2.27.0
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH] QSBR: Use xor operation to replace add operation when changing rcu_gp.ctr value

[Mathieu Desnoyers via lttng-dev]

----- On Feb 16, 2022, at 2:35 AM, lttng-dev lttng-dev@lists.lttng.org wrote:

> It is enough to have three values of rcu_gp.ctr, 00 for INACTIVE,
> 01 or 11 for ACTIVE. So it is possible to replace add operation
> with xor operation when changing rcu_gp.ctr value.

What is missing here is a description justifying why this change is useful.

What is inherently better about XOR compared to ADD or even binary-OR ?

If it's about performance, then a benchmark on relevant architectures
would be useful. But I suspect that if end users care that much about the
performance of urcu_qsbr_synchronize_rcu(), they might be doing something
wrong.

Thanks,

Mathieu

> 
> Signed-off-by: yaowenbin <yaowenbin1@huawei.com>
> ---
> src/urcu-qsbr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/urcu-qsbr.c b/src/urcu-qsbr.c
> index 3709412..46135f9 100644
> --- a/src/urcu-qsbr.c
> +++ b/src/urcu-qsbr.c
> @@ -391,7 +391,7 @@ void urcu_qsbr_synchronize_rcu(void)
> 		goto out;
> 
> 	/* Increment current G.P. */
> -	CMM_STORE_SHARED(urcu_qsbr_gp.ctr, urcu_qsbr_gp.ctr + URCU_QSBR_GP_CTR);
> +	CMM_STORE_SHARED(urcu_qsbr_gp.ctr, urcu_qsbr_gp.ctr ^ URCU_QSBR_GP_CTR);
> 
> 	/*
> 	 * Must commit urcu_qsbr_gp.ctr update to memory before waiting for
> --
> 2.27.0
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH] QSBR: Use xor operation to replace add operation when changing rcu_gp.ctr value

[Paul E. McKenney via lttng-dev]

On Wed, Feb 16, 2022 at 03:53:20PM -0500, Mathieu Desnoyers wrote:
> ----- On Feb 16, 2022, at 2:35 AM, lttng-dev lttng-dev@lists.lttng.org wrote:
> 
> > It is enough to have three values of rcu_gp.ctr, 00 for INACTIVE,
> > 01 or 11 for ACTIVE. So it is possible to replace add operation
> > with xor operation when changing rcu_gp.ctr value.
> 
> What is missing here is a description justifying why this change is useful.
> 
> What is inherently better about XOR compared to ADD or even binary-OR ?
> 
> If it's about performance, then a benchmark on relevant architectures
> would be useful. But I suspect that if end users care that much about the
> performance of urcu_qsbr_synchronize_rcu(), they might be doing something
> wrong.

Plus having the full counter can be extremely helpful when debugging.

							Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> > 
> > Signed-off-by: yaowenbin <yaowenbin1@huawei.com>
> > ---
> > src/urcu-qsbr.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/urcu-qsbr.c b/src/urcu-qsbr.c
> > index 3709412..46135f9 100644
> > --- a/src/urcu-qsbr.c
> > +++ b/src/urcu-qsbr.c
> > @@ -391,7 +391,7 @@ void urcu_qsbr_synchronize_rcu(void)
> > 		goto out;
> > 
> > 	/* Increment current G.P. */
> > -	CMM_STORE_SHARED(urcu_qsbr_gp.ctr, urcu_qsbr_gp.ctr + URCU_QSBR_GP_CTR);
> > +	CMM_STORE_SHARED(urcu_qsbr_gp.ctr, urcu_qsbr_gp.ctr ^ URCU_QSBR_GP_CTR);
> > 
> > 	/*
> > 	 * Must commit urcu_qsbr_gp.ctr update to memory before waiting for
> > --
> > 2.27.0
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev