Linux-arch Archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	elver@google.com, akpm@linux-foundation.org, tglx@linutronix.de,
	peterz@infradead.org, dianders@chromium.org, pmladek@suse.com,
	arnd@arndb.de, torvalds@linux-foundation.org,
	kernel-team@meta.com
Subject: Re: [PATCH v2 cmpxchg 09/13] lib: Add one-byte emulation function
Date: Mon, 13 May 2024 08:41:27 -0700	[thread overview]
Message-ID: <9f0ff126-2806-488e-97cc-7258eff0c574@paulmck-laptop> (raw)
In-Reply-To: <ZkInMNOsLO5XbDj5@boqun-archlinux>

On Mon, May 13, 2024 at 07:44:00AM -0700, Boqun Feng wrote:
> On Wed, May 01, 2024 at 04:01:26PM -0700, Paul E. McKenney wrote:
> > Architectures are required to provide four-byte cmpxchg() and 64-bit
> > architectures are additionally required to provide eight-byte cmpxchg().
> > However, there are cases where one-byte cmpxchg() would be extremely
> > useful.  Therefore, provide cmpxchg_emu_u8() that emulates one-byte
> > cmpxchg() in terms of four-byte cmpxchg().
> > 
> > Note that this emulations is fully ordered, and can (for example) cause
> > one-byte cmpxchg_relaxed() to incur the overhead of full ordering.
> > If this causes problems for a given architecture, that architecture is
> > free to provide its own lighter-weight primitives.
> > 
> > [ paulmck: Apply Marco Elver feedback. ]
> > [ paulmck: Apply kernel test robot feedback. ]
> > [ paulmck: Drop two-byte support per Arnd Bergmann feedback. ]
> > 
> > Link: https://lore.kernel.org/all/0733eb10-5e7a-4450-9b8a-527b97c842ff@paulmck-laptop/
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Acked-by: Marco Elver <elver@google.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Petr Mladek <pmladek@suse.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: <linux-arch@vger.kernel.org>
> > ---
> >  arch/Kconfig                |  3 +++
> >  include/linux/cmpxchg-emu.h | 15 +++++++++++++
> >  lib/Makefile                |  1 +
> >  lib/cmpxchg-emu.c           | 45 +++++++++++++++++++++++++++++++++++++
> >  4 files changed, 64 insertions(+)
> >  create mode 100644 include/linux/cmpxchg-emu.h
> >  create mode 100644 lib/cmpxchg-emu.c
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 9f066785bb71d..284663392eef8 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -1609,4 +1609,7 @@ config CC_HAS_SANE_FUNCTION_ALIGNMENT
> >  	# strict alignment always, even with -falign-functions.
> >  	def_bool CC_HAS_MIN_FUNCTION_ALIGNMENT || CC_IS_CLANG
> >  
> > +config ARCH_NEED_CMPXCHG_1_EMU
> > +	bool
> > +
> >  endmenu
> > diff --git a/include/linux/cmpxchg-emu.h b/include/linux/cmpxchg-emu.h
> > new file mode 100644
> > index 0000000000000..998deec67740a
> > --- /dev/null
> > +++ b/include/linux/cmpxchg-emu.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Emulated 1-byte and 2-byte cmpxchg operations for architectures
> > + * lacking direct support for these sizes.  These are implemented in terms
> > + * of 4-byte cmpxchg operations.
> > + *
> > + * Copyright (C) 2024 Paul E. McKenney.
> > + */
> > +
> > +#ifndef __LINUX_CMPXCHG_EMU_H
> > +#define __LINUX_CMPXCHG_EMU_H
> > +
> > +uintptr_t cmpxchg_emu_u8(volatile u8 *p, uintptr_t old, uintptr_t new);
> > +
> > +#endif /* __LINUX_CMPXCHG_EMU_H */
> > diff --git a/lib/Makefile b/lib/Makefile
> > index ffc6b2341b45a..cc3d52fdb477d 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -236,6 +236,7 @@ obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> >  lib-$(CONFIG_GENERIC_BUG) += bug.o
> >  
> >  obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o
> > +obj-$(CONFIG_ARCH_NEED_CMPXCHG_1_EMU) += cmpxchg-emu.o
> >  
> >  obj-$(CONFIG_DYNAMIC_DEBUG_CORE) += dynamic_debug.o
> >  #ensure exported functions have prototypes
> > diff --git a/lib/cmpxchg-emu.c b/lib/cmpxchg-emu.c
> > new file mode 100644
> > index 0000000000000..27f6f97cb60dd
> > --- /dev/null
> > +++ b/lib/cmpxchg-emu.c
> > @@ -0,0 +1,45 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Emulated 1-byte cmpxchg operation for architectures lacking direct
> > + * support for this size.  This is implemented in terms of 4-byte cmpxchg
> > + * operations.
> > + *
> > + * Copyright (C) 2024 Paul E. McKenney.
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <linux/export.h>
> > +#include <linux/instrumented.h>
> > +#include <linux/atomic.h>
> > +#include <linux/panic.h>
> > +#include <linux/bug.h>
> > +#include <asm-generic/rwonce.h>
> > +#include <linux/cmpxchg-emu.h>
> > +
> > +union u8_32 {
> > +	u8 b[4];
> > +	u32 w;
> > +};
> > +
> > +/* Emulate one-byte cmpxchg() in terms of 4-byte cmpxchg. */
> > +uintptr_t cmpxchg_emu_u8(volatile u8 *p, uintptr_t old, uintptr_t new)
> > +{
> > +	u32 *p32 = (u32 *)(((uintptr_t)p) & ~0x3);
> > +	int i = ((uintptr_t)p) & 0x3;
> > +	union u8_32 old32;
> > +	union u8_32 new32;
> > +	u32 ret;
> > +
> > +	ret = READ_ONCE(*p32);
> > +	do {
> > +		old32.w = ret;
> > +		if (old32.b[i] != old)
> > +			return old32.b[i];
> > +		new32.w = old32.w;
> > +		new32.b[i] = new;
> > +		instrument_atomic_read_write(p, 1);
> > +		ret = data_race(cmpxchg(p32, old32.w, new32.w)); // Overridden above.
> 
> Just out of curiosity, why is this `data_race` needed? cmpxchg is atomic
> so there should be no chance for a data race?

That is what I thought, too.  ;-)

The problem is that the cmpxchg() covers 32 bits, and so without that
data_race(), KCSAN would complain about data races with perfectly
legitimate concurrent accesses to the other three bytes.

The instrument_atomic_read_write(p, 1) beforehand tells KCSAN to complain
about concurrent accesses, but only to that one byte.

							Thanx, Paul

> Regards,
> Boqun
> 
> > +	} while (ret != old32.w);
> > +	return old;
> > +}
> > +EXPORT_SYMBOL_GPL(cmpxchg_emu_u8);
> > -- 
> > 2.40.1
> > 

  reply	other threads:[~2024-05-13 15:41 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-01 21:39 [PATCH RFC cmpxchg 0/8] Provide emulation for one- and two-byte cmpxchg() Paul E. McKenney
2024-04-01 21:39 ` [PATCH RFC cmpxchg 1/8] lib: Add one-byte and two-byte cmpxchg() emulation functions Paul E. McKenney
2024-04-02 13:07   ` Marco Elver
2024-04-02 17:15     ` Paul E. McKenney
2024-04-08 17:47 ` [PATCH RFC cmpxchg 0/8] Provide emulation for one- and two-byte cmpxchg() Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 01/14] sparc32: make __cmpxchg_u32() return u32 Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 02/14] sparc32: make the first argument of __cmpxchg_u64() volatile u64 * Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 03/14] sparc32: unify __cmpxchg_u{32,64} Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 04/14] sparc32: add __cmpxchg_u{8,16}() and teach __cmpxchg() to handle those sizes Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 05/14] parisc: __cmpxchg_u32(): lift conversion into the callers Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 06/14] parisc: unify implementations of __cmpxchg_u{8,32,64} Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 07/14] parisc: add missing export of __cmpxchg_u8() Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 08/14] parisc: add u16 support to cmpxchg() Paul E. McKenney
2024-04-08 20:10     ` Linus Torvalds
2024-04-08 20:53       ` Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 09/14] lib: Add one-byte emulation function Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 10/14] ARC: Emulate one-byte cmpxchg Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 11/14] csky: " Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 12/14] sh: " Paul E. McKenney
2024-04-18  8:04     ` Geert Uytterhoeven
2024-04-08 17:49   ` [PATCH cmpxchg 13/14] xtensa: " Paul E. McKenney
2024-04-18  8:06     ` Geert Uytterhoeven
2024-04-18 23:21       ` Paul E. McKenney
2024-04-19  5:07         ` Yujie Liu
2024-04-19  8:02           ` Geert Uytterhoeven
2024-04-20 14:03             ` Paul E. McKenney
2024-04-08 17:49   ` [PATCH cmpxchg 14/14] riscv: " Paul E. McKenney
2024-04-09 17:35     ` Andrea Parri
2024-04-09 18:08       ` Paul E. McKenney
2024-05-01 22:58   ` [PATCH v2 cmpxchg 0/8] Provide emulation for one--byte cmpxchg() Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 01/13] sparc32: make __cmpxchg_u32() return u32 Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 02/13] sparc32: make the first argument of __cmpxchg_u64() volatile u64 * Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 03/13] sparc32: unify __cmpxchg_u{32,64} Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 04/13] sparc32: add __cmpxchg_u{8,16}() and teach __cmpxchg() to handle those sizes Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 05/13] parisc: __cmpxchg_u32(): lift conversion into the callers Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 06/13] parisc: unify implementations of __cmpxchg_u{8,32,64} Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 07/13] parisc: add missing export of __cmpxchg_u8() Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 08/13] parisc: add u16 support to cmpxchg() Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 09/13] lib: Add one-byte emulation function Paul E. McKenney
2024-05-13 14:44       ` Boqun Feng
2024-05-13 15:41         ` Paul E. McKenney [this message]
2024-05-13 15:57           ` Boqun Feng
2024-05-13 21:19             ` Boqun Feng
2024-05-14 14:22               ` Paul E. McKenney
2024-05-14 14:53                 ` Boqun Feng
2024-05-14 15:02                   ` Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 10/13] ARC: Emulate one-byte cmpxchg Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 11/13] csky: " Paul E. McKenney
2024-05-11  6:42       ` Guo Ren
2024-05-11 14:49         ` Paul E. McKenney
2024-05-01 23:01     ` [PATCH v2 cmpxchg 12/13] sh: " Paul E. McKenney
2024-05-02  4:52       ` John Paul Adrian Glaubitz
2024-05-02  5:06         ` Paul E. McKenney
2024-05-02  5:11           ` John Paul Adrian Glaubitz
2024-05-02 13:33             ` Paul E. McKenney
2024-05-02 20:53               ` Al Viro
2024-05-02 21:01                 ` alpha cmpxchg.h (was Re: [PATCH v2 cmpxchg 12/13] sh: Emulate one-byte cmpxchg) Al Viro
2024-05-02 22:16                   ` Linus Torvalds
2024-05-02 21:18                 ` [PATCH v2 cmpxchg 12/13] sh: Emulate one-byte cmpxchg Paul E. McKenney
2024-05-02 22:07                   ` Al Viro
2024-05-02 23:12                     ` Paul E. McKenney
2024-05-02 23:24                       ` Al Viro
2024-05-02 23:45                         ` Paul E. McKenney
2024-05-02 23:32                       ` Linus Torvalds
2024-05-03  0:16                         ` Paul E. McKenney
2024-05-02 21:50               ` Arnd Bergmann
2024-05-02  5:42           ` D. Jeff Dionne
2024-05-02 11:30             ` Arnd Bergmann
2024-05-01 23:01     ` [PATCH v2 cmpxchg 13/13] xtensa: " Paul E. McKenney
2024-05-02 20:01     ` [PATCH v2 cmpxchg 0/8] Provide emulation for one--byte cmpxchg() Al Viro
2024-05-02 21:20       ` Paul E. McKenney
2024-06-04 17:02     ` [PATCH v3 cmpxchg 0/4] " Paul E. McKenney
2024-06-04 17:04       ` [PATCH v3 cmpxchg 1/4] ARC: Emulate one-byte cmpxchg Paul E. McKenney
2024-06-04 17:04       ` [PATCH v3 cmpxchg 2/4] sh: " Paul E. McKenney
2024-06-04 17:09         ` John Paul Adrian Glaubitz
2024-06-04 17:50           ` Paul E. McKenney
2024-06-04 17:56             ` John Paul Adrian Glaubitz
2024-06-04 21:14               ` Paul E. McKenney
2024-06-04 17:04       ` [PATCH v3 cmpxchg 3/4] xtensa: " Paul E. McKenney
2024-06-04 17:04       ` [PATCH v3 cmpxchg 4/4] ARM: " Paul E. McKenney
2024-06-04 20:52         ` Linus Walleij
2024-06-04 21:14           ` Paul E. McKenney
2024-06-05  8:38             ` Linus Walleij
2024-06-05 18:05               ` Paul E. McKenney

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=9f0ff126-2806-488e-97cc-7258eff0c574@paulmck-laptop \
    --to=paulmck@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=boqun.feng@gmail.com \
    --cc=dianders@chromium.org \
    --cc=elver@google.com \
    --cc=kernel-team@meta.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).