Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
From: "Paul E. McKenney via lttng-dev" <lttng-dev@lists.lttng.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Olivier Dion <odion@efficios.com>, lttng-dev@lists.lttng.org
Subject: Re: [lttng-dev] [PATCH 02/11] urcu/uatomic: Use atomic builtins if configured
Date: Thu, 22 Jun 2023 11:32:57 -0700	[thread overview]
Message-ID: <105bad2a-9fed-4353-a434-a571ce9da7ac@paulmck-laptop> (raw)
In-Reply-To: <ffec193e-7060-3fe2-6526-c54e89e992e6@efficios.com>

On Thu, Jun 22, 2023 at 11:55:55AM -0400, Mathieu Desnoyers wrote:
> On 6/21/23 19:19, Paul E. McKenney wrote:
> [...]
> > > diff --git a/include/urcu/uatomic/builtins-generic.h b/include/urcu/uatomic/builtins-generic.h
> > > new file mode 100644
> > > index 0000000..8e6a9b5
> > > --- /dev/null
> > > +++ b/include/urcu/uatomic/builtins-generic.h
> > > @@ -0,0 +1,85 @@
> > > +/*
> > > + * urcu/uatomic/builtins-generic.h
> > > + *
> > > + * Copyright (c) 2023 Olivier Dion <odion@efficios.com>
> > > + *
> > > + * This library is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU Lesser General Public
> > > + * License as published by the Free Software Foundation; either
> > > + * version 2.1 of the License, or (at your option) any later version.
> > > + *
> > > + * This library is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > + * Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public
> > > + * License along with this library; if not, write to the Free Software
> > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > + */
> > > +
> > > +#ifndef _URCU_UATOMIC_BUILTINS_GENERIC_H
> > > +#define _URCU_UATOMIC_BUILTINS_GENERIC_H
> > > +
> > > +#include <urcu/system.h>
> > > +
> > > +#define uatomic_set(addr, v) __atomic_store_n(addr, v, __ATOMIC_RELAXED)
> > > +
> > > +#define uatomic_read(addr) __atomic_load_n(addr, __ATOMIC_RELAXED)
> > 
> > Does this lose the volatile semantics that the old-style definitions
> > had?
> > 
> 
> Yes.
> 
> [...]
> 
> > > +++ b/include/urcu/uatomic/builtins-x86.h
> > > @@ -0,0 +1,85 @@
> > > +/*
> > > + * urcu/uatomic/builtins-x86.h
> > > + *
> > > + * Copyright (c) 2023 Olivier Dion <odion@efficios.com>
> > > + *
> > > + * This library is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU Lesser General Public
> > > + * License as published by the Free Software Foundation; either
> > > + * version 2.1 of the License, or (at your option) any later version.
> > > + *
> > > + * This library is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > + * Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public
> > > + * License along with this library; if not, write to the Free Software
> > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > + */
> > > +
> > > +#ifndef _URCU_UATOMIC_BUILTINS_X86_H
> > > +#define _URCU_UATOMIC_BUILTINS_X86_H
> > > +
> > > +#include <urcu/system.h>
> > > +
> > > +#define uatomic_set(addr, v) __atomic_store_n(addr, v, __ATOMIC_RELAXED)
> > > +
> > > +#define uatomic_read(addr) __atomic_load_n(addr, __ATOMIC_RELAXED)
> > 
> > And same question here.
> 
> Yes, this opens interesting questions:
> 
> * what semantic do we want for uatomic_read/set ?
> 
> * what semantic do we want for CMM_LOAD_SHARED/CMM_STORE_SHARED ?
> 
> * do we want to allow load/store-shared to work on variables larger than a
> word ? (e.g. on a uint64_t on a 32-bit architecture, or on a structure)
> 
> * what are the guarantees of a volatile type ?
> 
> * what are the guarantees of a load/store relaxed in C11 ?
> 
> Does the delta between volatile and C11 relaxed guarantees matter ?
> 
> Is there an advantage to use C11 load/store relaxed over volatile ? Should
> we combine both C11 load/store _and_ volatile ? Should we use
> atomic_signal_fence instead ?

I suggest C11 volatile atomic load/store.  Load/store fusing is permitted
for non-volatile atomic loads and stores, and such fusing can ruin your
code's entire day.  ;-)

							Thanx, Paul
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

  reply	other threads:[~2023-06-22 18:33 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-15 20:17 [lttng-dev] [PATCH 00/11] Add support for TSAN to liburcu Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 01/11] configure: Add --disable-atomic-builtins option Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 02/11] urcu/uatomic: Use atomic builtins if configured Olivier Dion via lttng-dev
2023-06-21 23:19   ` Paul E. McKenney via lttng-dev
2023-06-22 15:55     ` Mathieu Desnoyers via lttng-dev
2023-06-22 18:32       ` Paul E. McKenney via lttng-dev [this message]
2023-06-22 19:53         ` Olivier Dion via lttng-dev
2023-06-22 19:56           ` Mathieu Desnoyers via lttng-dev
2023-06-22 20:10             ` Olivier Dion via lttng-dev
2023-06-22 20:11           ` Paul E. McKenney via lttng-dev
2023-06-22 19:54         ` Mathieu Desnoyers via lttng-dev
2023-06-29 17:22         ` Olivier Dion via lttng-dev
2023-06-29 17:27           ` Olivier Dion via lttng-dev
2023-06-29 18:33             ` Mathieu Desnoyers via lttng-dev
2023-06-29 18:29           ` Mathieu Desnoyers via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 03/11] urcu/compiler: " Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 04/11] urcu/arch/generic: " Olivier Dion via lttng-dev
2023-06-21 23:22   ` Paul E. McKenney via lttng-dev
2023-06-22  0:53     ` Olivier Dion via lttng-dev
2023-06-22  1:48       ` Mathieu Desnoyers via lttng-dev
2023-06-22  3:44         ` Paul E. McKenney via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 05/11] urcu/system: " Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 06/11] urcu/uatomic: Add CMM memory model Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 07/11] urcu-wait: Fix wait state load/store Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 08/11] tests: Use uatomic for accessing global states Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 09/11] benchmark: " Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 10/11] tests/unit/test_build: Quiet unused return value Olivier Dion via lttng-dev
2023-05-15 20:17 ` [lttng-dev] [PATCH 11/11] urcu/annotate: Add CMM annotation Olivier Dion via lttng-dev
2023-05-16 15:57   ` Olivier Dion via lttng-dev
2023-05-16  8:18 ` [lttng-dev] [PATCH 00/11] Add support for TSAN to liburcu Dmitry Vyukov via lttng-dev
2023-05-16 15:47   ` Olivier Dion via lttng-dev
2023-05-17 10:21     ` Dmitry Vyukov via lttng-dev
2023-05-17 10:57       ` Dmitry Vyukov via lttng-dev
2023-05-17 14:44         ` Olivier Dion via lttng-dev
2023-05-23 16:05           ` Olivier Dion via lttng-dev
2023-05-24  8:14             ` Dmitry Vyukov via lttng-dev
2023-05-26  5:33               ` Ondřej Surý via lttng-dev
2023-05-26  6:08                 ` Dmitry Vyukov via lttng-dev
2023-05-26  6:10                   ` Dmitry Vyukov via lttng-dev
2023-05-26 10:06                   ` Ondřej Surý via lttng-dev
2023-05-26 10:08                     ` Dmitry Vyukov via lttng-dev
2023-05-26 14:20                     ` Olivier Dion via lttng-dev
2023-05-26 15:15               ` Olivier Dion via lttng-dev
2023-05-17 14:44       ` Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 00/12] " Olivier Dion via lttng-dev
2023-06-07 19:04   ` Ondřej Surý via lttng-dev
2023-06-07 19:20     ` Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 01/12] configure: Add --disable-atomic-builtins option Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 02/12] urcu/compiler: Use atomic builtins if configured Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 03/12] urcu/arch/generic: " Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 04/12] urcu/system: " Olivier Dion via lttng-dev
2023-06-21 23:23   ` Paul E. McKenney via lttng-dev
2023-07-04 14:43     ` Olivier Dion via lttng-dev
2023-07-05 18:48       ` Paul E. McKenney via lttng-dev
2023-07-05 19:03         ` Olivier Dion via lttng-dev
2023-07-05 19:28           ` Paul E. McKenney via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 05/12] urcu/uatomic: Add CMM memory model Olivier Dion via lttng-dev
2023-06-21 23:28   ` Paul E. McKenney via lttng-dev
2023-06-29 16:49     ` Olivier Dion via lttng-dev
2023-06-29 18:40       ` Paul E. McKenney via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 06/12] urcu-wait: Fix wait state load/store Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 07/12] tests: Use uatomic for accessing global states Olivier Dion via lttng-dev
2023-06-21 23:37   ` Paul E. McKenney via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 08/12] benchmark: " Olivier Dion via lttng-dev
2023-06-21 23:38   ` Paul E. McKenney via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 09/12] tests/unit/test_build: Quiet unused return value Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 10/12] urcu/annotate: Add CMM annotation Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 11/12] Add cmm_emit_legacy_smp_mb() Olivier Dion via lttng-dev
2023-06-07 18:53 ` [lttng-dev] [PATCH v2 12/12] tests: Add tests for checking race conditions Olivier Dion via lttng-dev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=105bad2a-9fed-4353-a434-a571ce9da7ac@paulmck-laptop \
    --to=lttng-dev@lists.lttng.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=odion@efficios.com \
    --cc=paulmck@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox