Re: [PATCH v1.1 6/6] libxfs: fix atomic64_t detection on x86 32-bit architectures

[Carlos Maiolino <cem@kernel.org>]

On Tue, Sep 12, 2023 at 12:47:51PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xfsprogs during compilation tries to detect if liburcu supports atomic
> 64-bit ops on the platform it is being compiled on, and if not it falls
> back to using pthread mutex locks.
> 
> The detection logic for that fallback relies on _uatomic_link_error()
> which is a link-time trick used by liburcu that will cause compilation
> errors on archs that lack the required support. That only works for the
> generic liburcu code though, and it is not implemented for the
> x86-specific code.
> 
> In practice this means that when xfsprogs is compiled on 32-bit x86
> archs will successfully link to liburcu for atomic ops, but liburcu does
> not support atomic64_t on those archs. It indicates this during runtime
> by generating an illegal instruction that aborts execution, and thus
> causes various xfsprogs utils to be segfaulting.
> 
> Fix this by requiring that unsigned longs are at least 64 bits in size,
> which /usually/ means that 64-bit atomic counters are supported.  We
> can't simply execute the liburcu atomic64_t detection code during
> configure instead of only relying on the linker error because that
> doesn't work for cross-compiled packages.
> 
> Fixes: 7448af588a2e ("libxfs: fix atomic64_t poorly for 32-bit architectures")
> Reported-by: Anthony Iliopoulos <ailiop@suse.com>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> v1.1: This time with correct commit message.
> ---

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Carlos

>  m4/package_urcu.m4 |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/m4/package_urcu.m4 b/m4/package_urcu.m4
> index ef116e0cda7..4bb2b886f06 100644
> --- a/m4/package_urcu.m4
> +++ b/m4/package_urcu.m4
> @@ -26,7 +26,11 @@ rcu_init();
>  #
>  # 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.
> +# support atomic operations on 64-bit data types for its generic
> +# implementation (which relies on compiler builtins). For certain archs
> +# where liburcu carries its own implementation (such as x86_32), it
> +# signals lack of support during runtime by emitting an illegal
> +# instruction, so we also need to check CAA_BITS_PER_LONG to detect that.
>  #
>  AC_DEFUN([AC_HAVE_LIBURCU_ATOMIC64],
>    [ AC_MSG_CHECKING([for atomic64_t support in liburcu])
> @@ -34,8 +38,11 @@ AC_DEFUN([AC_HAVE_LIBURCU_ATOMIC64],
>      [	AC_LANG_PROGRAM([[
>  #define _GNU_SOURCE
>  #include <urcu.h>
> +#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>  	]], [[
>  long long f = 3;
> +
> +BUILD_BUG_ON(CAA_BITS_PER_LONG < 64);
>  uatomic_inc(&f);
>  	]])
>      ], have_liburcu_atomic64=yes