]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
libxfs: fix atomic64_t poorly for 32-bit architectures
authorDarrick J. Wong <djwong@kernel.org>
Thu, 2 Dec 2021 20:24:33 +0000 (15:24 -0500)
committerEric Sandeen <sandeen@sandeen.net>
Thu, 2 Dec 2021 20:24:33 +0000 (15:24 -0500)
In commit de555f66, we converted the atomic64_t implementation to use
the liburcu uatomic_* functions.  Regrettably, nobody tried to build
xfsprogs on a 32-bit architecture (hint: maintainers don't scale well
anymore) so nobody noticed that the build fails due to the unknown
symbol _uatomic_link_error.  This is what happens when liburcu doesn't
know how to perform atomic updates to a variable of a certain size, due
to some horrid macro magic in urcu.h.

Rather than a strict revert to non-atomic updates for these platforms or
(which would introduce a landmine) or roll everything back for the sake
of older platforms, I went with providing a custom atomic64_t
implementation that uses a single pthread mutex.  This enables us to
work around the fact that the kernel atomic64_t API doesn't require a
special initializer function, and is probably good enough since there
are only a handful of atomic64_t counters in the kernel.

Clean up the type declarations of a couple of variables in libxlog to
match the kernel usage, though that's probably overkill.

Eventually we'll want to decide if we're deprecating 32-bit, but this
fixes them in the mean time.

[sandeen: make the custom atomic64_add() take an int64_t]

Fixes: de555f66 ("atomic: convert to uatomic")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
configure.ac
include/atomic.h
include/builddefs.in
include/libxlog.h
libxfs/init.c
m4/package_urcu.m4
repair/phase2.c

index 89a53170d6816f20301093694d06d4401f1bbd60..6adbee8c31aa1a09640c496ed675e3472e3e282f 100644 (file)
@@ -238,6 +238,7 @@ AC_CHECK_SIZEOF([long])
 AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T
 AC_MANUAL_FORMAT
+AC_HAVE_LIBURCU_ATOMIC64
 
 AC_CONFIG_FILES([include/builddefs])
 AC_OUTPUT
index 2804815eea5f3eb2a08bfb29753dd447ab1d3b09..dc6438a3a4f69d112db9129f2ef00b6f34e1e6cc 100644 (file)
@@ -60,11 +60,57 @@ static inline bool atomic_dec_and_lock(atomic_t *a, spinlock_t *lock)
        return 0;
 }
 
+#ifdef HAVE_LIBURCU_ATOMIC64
+/*
+ * On most (64-bit) platforms, liburcu can handle 64-bit atomic counter
+ * updates, so we preferentially use that.
+ */
 #define atomic64_read(a)       uatomic_read(a)
 #define atomic64_set(a, v)     uatomic_set(a, v)
 #define atomic64_add(v, a)     uatomic_add(a, v)
 #define atomic64_sub(v, a)     uatomic_sub(a, v)
 #define atomic64_inc(a)                uatomic_inc(a)
 #define atomic64_dec(a)                uatomic_dec(a)
+#else
+/*
+ * If we don't detect support for that, emulate it with a lock.  Currently
+ * there are only three atomic64_t counters in userspace and none of them are
+ * performance critical, so we serialize them all with a single mutex since
+ * the kernel atomic64_t API doesn't have an _init call.
+ */
+extern pthread_mutex_t atomic64_lock;
+
+static inline int64_t
+atomic64_read(atomic64_t *a)
+{
+       int64_t ret;
+
+       pthread_mutex_lock(&atomic64_lock);
+       ret = *a;
+       pthread_mutex_unlock(&atomic64_lock);
+       return ret;
+}
+
+static inline void
+atomic64_add(int64_t v, atomic64_t *a)
+{
+       pthread_mutex_lock(&atomic64_lock);
+       (*a) += v;
+       pthread_mutex_unlock(&atomic64_lock);
+}
+
+static inline void
+atomic64_set(atomic64_t *a, int64_t v)
+{
+       pthread_mutex_lock(&atomic64_lock);
+       (*a) = v;
+       pthread_mutex_unlock(&atomic64_lock);
+}
+
+#define atomic64_inc(a)                atomic64_add(1, (a))
+#define atomic64_dec(a)                atomic64_add(-1, (a))
+#define atomic64_sub(v, a)     atomic64_add(-(v), (a))
+
+#endif /* HAVE_URCU_ATOMIC64 */
 
 #endif /* __ATOMIC_H__ */
index 78eddf4a9852fc8df252bcae113a283d331356fe..9d0b080053bf0d6870acf26a6c656ec60cacf4e8 100644 (file)
@@ -122,6 +122,7 @@ HAVE_SYSTEMD = @have_systemd@
 SYSTEMD_SYSTEM_UNIT_DIR = @systemd_system_unit_dir@
 HAVE_CROND = @have_crond@
 CROND_DIR = @crond_dir@
+HAVE_LIBURCU_ATOMIC64 = @have_liburcu_atomic64@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #         -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
@@ -159,6 +160,9 @@ endif
 
 LIBICU_LIBS = @libicu_LIBS@
 LIBICU_CFLAGS = @libicu_CFLAGS@
+ifeq ($(HAVE_LIBURCU_ATOMIC64),yes)
+PCFLAGS += -DHAVE_LIBURCU_ATOMIC64
+endif
 
 SANITIZER_CFLAGS += @addrsan_cflags@ @threadsan_cflags@ @ubsan_cflags@
 SANITIZER_LDFLAGS += @addrsan_ldflags@ @threadsan_ldflags@ @ubsan_ldflags@
index adaa9963cddcfde57b0e35db15222ddff77bf463..3ade7ffaf5a3be7614fc068d7f2ded6013f7685c 100644 (file)
@@ -11,8 +11,8 @@
  * the need to define any exotic kernel types in userland.
  */
 struct xlog {
-       xfs_lsn_t       l_tail_lsn;     /* lsn of 1st LR w/ unflush buffers */
-       xfs_lsn_t       l_last_sync_lsn;/* lsn of last LR on disk */
+       atomic64_t      l_tail_lsn;     /* lsn of 1st LR w/ unflush buffers */
+       atomic64_t      l_last_sync_lsn;/* lsn of last LR on disk */
        xfs_mount_t     *l_mp;          /* mount point */
        struct xfs_buftarg *l_dev;              /* dev_t of log */
        xfs_daddr_t     l_logBBstart;   /* start block of log */
index 0f7e89509d5f77ccb88846228015ac352b47a007..75ff4d49f5adf0486c09d41dbddc8e36c7443d59 100644 (file)
 
 #include "libxfs.h"            /* for now */
 
+#ifndef HAVE_LIBURCU_ATOMIC64
+pthread_mutex_t        atomic64_lock = PTHREAD_MUTEX_INITIALIZER;
+#endif
+
 char *progname = "libxfs";     /* default, changed by each tool */
 
 struct cache *libxfs_bcache;   /* global buffer cache */
index f0337f341e56223a79e34e3b3ccc3bb8d0c8f5cb..f8e798b661362e24449b928957d6bed0c68c0059 100644 (file)
@@ -20,3 +20,22 @@ AC_DEFUN([AC_PACKAGE_NEED_RCU_INIT],
        AC_MSG_RESULT(no))
     AC_SUBST(liburcu)
   ])
+
+#
+# Make sure that calling uatomic_inc on a 64-bit integer doesn't cause a link
+# error on _uatomic_link_error, which is how liburcu signals that it doesn't
+# support atomic operations on 64-bit data types.
+#
+AC_DEFUN([AC_HAVE_LIBURCU_ATOMIC64],
+  [ AC_MSG_CHECKING([for atomic64_t support in liburcu])
+    AC_TRY_LINK([
+#define _GNU_SOURCE
+#include <urcu.h>
+    ], [
+       long long f = 3;
+       uatomic_inc(&f);
+    ], have_liburcu_atomic64=yes
+       AC_MSG_RESULT(yes),
+       AC_MSG_RESULT(no))
+    AC_SUBST(have_liburcu_atomic64)
+  ])
index cb9adf1d3db5dc0c1f2dcec696f4374e48be0f76..32ffe18b301d9ff44c70aad602e6c7a438b5df71 100644 (file)
@@ -128,7 +128,7 @@ zero_log(
         * is a v5 filesystem.
         */
        if (xfs_sb_version_hascrc(&mp->m_sb))
-               libxfs_max_lsn = log->l_last_sync_lsn;
+               libxfs_max_lsn = atomic64_read(&log->l_last_sync_lsn);
 }
 
 static bool