* [PATCHSET 0/2] xfs: fixes for 5.14.1
@ 2021-11-23 19:53 Darrick J. Wong
2021-11-23 19:53 ` [PATCH 1/2] libfrog: fix crc32c self test code on cross builds Darrick J. Wong
2021-11-23 19:53 ` [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures Darrick J. Wong
0 siblings, 2 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-11-23 19:53 UTC (permalink / raw)
To: sandeen, djwong; +Cc: Bastian Germann, Helmut Grohne, Dave Chinner, linux-xfs
Hi all,
This quick series fixes some build problems in xfsprogs 5.14.0.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
--D
xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=xfsprogs-5.14-fixes
---
configure.ac | 1 +
include/atomic.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/builddefs.in | 4 ++++
include/libxlog.h | 4 ++--
libfrog/crc32.c | 7 ++++++-
libxfs/init.c | 4 ++++
m4/package_urcu.m4 | 19 +++++++++++++++++++
repair/phase2.c | 2 +-
8 files changed, 83 insertions(+), 4 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] libfrog: fix crc32c self test code on cross builds
2021-11-23 19:53 [PATCHSET 0/2] xfs: fixes for 5.14.1 Darrick J. Wong
@ 2021-11-23 19:53 ` Darrick J. Wong
2021-11-23 23:32 ` Eric Sandeen
2021-11-23 19:53 ` [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures Darrick J. Wong
1 sibling, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2021-11-23 19:53 UTC (permalink / raw)
To: sandeen, djwong; +Cc: Helmut Grohne, Bastian Germann, Dave Chinner, linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
Helmut Grohne reported that the crc32c self test program fails to cross
build on 5.14.0 if the build host doesn't have liburcu installed. We
don't need userspace RCU functionality to test crc32 on the build host,
so twiddle the header files to include only the two header files that we
actually need.
Note: Build-time testing of crc32c is useful for upstream developers so
that we can check that we haven't broken the checksum code, but we
really ought to be testing this in mkfs and repair on the user's system
so that they don't end up with garbage filesystems. A future patch will
introduce that.
Reported-by: Helmut Grohne <helmut@subdivi.de>
Cc: Bastian Germann <bage@debian.org>
Suggested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
libfrog/crc32.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libfrog/crc32.c b/libfrog/crc32.c
index 526ce950..6a273b71 100644
--- a/libfrog/crc32.c
+++ b/libfrog/crc32.c
@@ -29,10 +29,15 @@
* match the hardware acceleration available on Intel CPUs.
*/
+/*
+ * Do not include platform_defs.h here; this will break cross builds if the
+ * build host does not have liburcu-dev installed.
+ */
+#include <stdio.h>
+#include <sys/types.h>
#include <inttypes.h>
#include <asm/types.h>
#include <sys/time.h>
-#include "platform_defs.h"
/* For endian conversion routines */
#include "xfs_arch.h"
#include "crc32defs.h"
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures
2021-11-23 19:53 [PATCHSET 0/2] xfs: fixes for 5.14.1 Darrick J. Wong
2021-11-23 19:53 ` [PATCH 1/2] libfrog: fix crc32c self test code on cross builds Darrick J. Wong
@ 2021-11-23 19:53 ` Darrick J. Wong
2021-11-24 2:21 ` Darrick J. Wong
2021-11-24 22:03 ` Eric Sandeen
1 sibling, 2 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-11-23 19:53 UTC (permalink / raw)
To: sandeen, djwong; +Cc: linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
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.
Fixes: de555f66 ("atomic: convert to uatomic")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
configure.ac | 1 +
include/atomic.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/builddefs.in | 4 ++++
include/libxlog.h | 4 ++--
libxfs/init.c | 4 ++++
m4/package_urcu.m4 | 19 +++++++++++++++++++
repair/phase2.c | 2 +-
7 files changed, 77 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index 89a53170..6adbee8c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -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
diff --git a/include/atomic.h b/include/atomic.h
index 2804815e..79e58dfe 100644
--- a/include/atomic.h
+++ b/include/atomic.h
@@ -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(int 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__ */
diff --git a/include/builddefs.in b/include/builddefs.in
index 78eddf4a..9d0b0800 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -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@
diff --git a/include/libxlog.h b/include/libxlog.h
index adaa9963..3ade7ffa 100644
--- a/include/libxlog.h
+++ b/include/libxlog.h
@@ -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 */
diff --git a/libxfs/init.c b/libxfs/init.c
index 0f7e8950..75ff4d49 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -25,6 +25,10 @@
#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 */
diff --git a/m4/package_urcu.m4 b/m4/package_urcu.m4
index f0337f34..f8e798b6 100644
--- a/m4/package_urcu.m4
+++ b/m4/package_urcu.m4
@@ -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)
+ ])
diff --git a/repair/phase2.c b/repair/phase2.c
index cb9adf1d..32ffe18b 100644
--- a/repair/phase2.c
+++ b/repair/phase2.c
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] libfrog: fix crc32c self test code on cross builds
2021-11-23 19:53 ` [PATCH 1/2] libfrog: fix crc32c self test code on cross builds Darrick J. Wong
@ 2021-11-23 23:32 ` Eric Sandeen
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2021-11-23 23:32 UTC (permalink / raw)
To: Darrick J. Wong, sandeen
Cc: Helmut Grohne, Bastian Germann, Dave Chinner, linux-xfs
On 11/23/21 1:53 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Helmut Grohne reported that the crc32c self test program fails to cross
> build on 5.14.0 if the build host doesn't have liburcu installed. We
> don't need userspace RCU functionality to test crc32 on the build host,
> so twiddle the header files to include only the two header files that we
> actually need.
>
> Note: Build-time testing of crc32c is useful for upstream developers so
> that we can check that we haven't broken the checksum code, but we
> really ought to be testing this in mkfs and repair on the user's system
> so that they don't end up with garbage filesystems. A future patch will
> introduce that.
>
> Reported-by: Helmut Grohne <helmut@subdivi.de>
> Cc: Bastian Germann <bage@debian.org>
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
LGTM, thanks. Helmut, can you confirm that this solves all cross-build problems?
(It must, but confirmation that the cross build is completely functional now
would be nice.)
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> ---
> libfrog/crc32.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>
> diff --git a/libfrog/crc32.c b/libfrog/crc32.c
> index 526ce950..6a273b71 100644
> --- a/libfrog/crc32.c
> +++ b/libfrog/crc32.c
> @@ -29,10 +29,15 @@
> * match the hardware acceleration available on Intel CPUs.
> */
>
> +/*
> + * Do not include platform_defs.h here; this will break cross builds if the
> + * build host does not have liburcu-dev installed.
> + */
> +#include <stdio.h>
> +#include <sys/types.h>
> #include <inttypes.h>
> #include <asm/types.h>
> #include <sys/time.h>
> -#include "platform_defs.h"
> /* For endian conversion routines */
> #include "xfs_arch.h"
> #include "crc32defs.h"
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures
2021-11-23 19:53 ` [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures Darrick J. Wong
@ 2021-11-24 2:21 ` Darrick J. Wong
2021-11-24 22:03 ` Eric Sandeen
1 sibling, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-11-24 2:21 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
On Tue, Nov 23, 2021 at 11:53:59AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> 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.
>
> Fixes: de555f66 ("atomic: convert to uatomic")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> configure.ac | 1 +
> include/atomic.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> include/builddefs.in | 4 ++++
> include/libxlog.h | 4 ++--
> libxfs/init.c | 4 ++++
> m4/package_urcu.m4 | 19 +++++++++++++++++++
> repair/phase2.c | 2 +-
> 7 files changed, 77 insertions(+), 3 deletions(-)
>
>
> diff --git a/configure.ac b/configure.ac
> index 89a53170..6adbee8c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -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
> diff --git a/include/atomic.h b/include/atomic.h
> index 2804815e..79e58dfe 100644
> --- a/include/atomic.h
> +++ b/include/atomic.h
> @@ -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(int v, atomic64_t *a)
fmeh, this should be int64_t for consistency. I'll respin this if the
maintainer wants me to?
--D
> +{
> + 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__ */
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 78eddf4a..9d0b0800 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -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@
> diff --git a/include/libxlog.h b/include/libxlog.h
> index adaa9963..3ade7ffa 100644
> --- a/include/libxlog.h
> +++ b/include/libxlog.h
> @@ -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 */
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 0f7e8950..75ff4d49 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -25,6 +25,10 @@
>
> #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 */
> diff --git a/m4/package_urcu.m4 b/m4/package_urcu.m4
> index f0337f34..f8e798b6 100644
> --- a/m4/package_urcu.m4
> +++ b/m4/package_urcu.m4
> @@ -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)
> + ])
> diff --git a/repair/phase2.c b/repair/phase2.c
> index cb9adf1d..32ffe18b 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures
2021-11-23 19:53 ` [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures Darrick J. Wong
2021-11-24 2:21 ` Darrick J. Wong
@ 2021-11-24 22:03 ` Eric Sandeen
1 sibling, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2021-11-24 22:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 11/23/21 1:53 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> 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.
>
> Fixes: de555f66 ("atomic: convert to uatomic")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
After a brief sidebar w/ Darrick I think we should change atomic64_add to take
an int64_t, I can fix that on commit. It shouldn't matter for our current usage.
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> ---
> configure.ac | 1 +
> include/atomic.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> include/builddefs.in | 4 ++++
> include/libxlog.h | 4 ++--
> libxfs/init.c | 4 ++++
> m4/package_urcu.m4 | 19 +++++++++++++++++++
> repair/phase2.c | 2 +-
> 7 files changed, 77 insertions(+), 3 deletions(-)
>
>
> diff --git a/configure.ac b/configure.ac
> index 89a53170..6adbee8c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -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
> diff --git a/include/atomic.h b/include/atomic.h
> index 2804815e..79e58dfe 100644
> --- a/include/atomic.h
> +++ b/include/atomic.h
> @@ -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(int 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__ */
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 78eddf4a..9d0b0800 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -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@
> diff --git a/include/libxlog.h b/include/libxlog.h
> index adaa9963..3ade7ffa 100644
> --- a/include/libxlog.h
> +++ b/include/libxlog.h
> @@ -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 */
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 0f7e8950..75ff4d49 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -25,6 +25,10 @@
>
> #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 */
> diff --git a/m4/package_urcu.m4 b/m4/package_urcu.m4
> index f0337f34..f8e798b6 100644
> --- a/m4/package_urcu.m4
> +++ b/m4/package_urcu.m4
> @@ -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)
> + ])
> diff --git a/repair/phase2.c b/repair/phase2.c
> index cb9adf1d..32ffe18b 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-24 22:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-23 19:53 [PATCHSET 0/2] xfs: fixes for 5.14.1 Darrick J. Wong
2021-11-23 19:53 ` [PATCH 1/2] libfrog: fix crc32c self test code on cross builds Darrick J. Wong
2021-11-23 23:32 ` Eric Sandeen
2021-11-23 19:53 ` [PATCH 2/2] libxfs: fix atomic64_t poorly for 32-bit architectures Darrick J. Wong
2021-11-24 2:21 ` Darrick J. Wong
2021-11-24 22:03 ` Eric Sandeen
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 NNTP newsgroup(s).