[PATCH] erofs-utils: lib: introduce atomic operations

[Yifan Zhao]

On 2/28/24 16:21, Gao Xiang wrote:
> Add some helpers (relaxed semantics) in order to prepare for the
> upcoming multi-threaded support.
>
> For example, compressor may be initialized more than once in different
> worker threads, resulting in noisy warnings.
>
> This patch makes sure that each message will be printed only once by
> adding `__warnonce` atomic booleans to each erofs_compressor_init().
>
> Cc: Yifan Zhao<zhaoyifan at sjtu.edu.cn>
> Signed-off-by: Gao Xiang<hsiangkao at linux.alibaba.com>
> ---
>   include/erofs/atomic.h      | 27 +++++++++++++++++++++++++++
>   lib/compressor_deflate.c    | 11 ++++++++---
>   lib/compressor_libdeflate.c |  6 +++++-
>   lib/compressor_liblzma.c    |  5 ++++-
>   4 files changed, 44 insertions(+), 5 deletions(-)
>   create mode 100644 include/erofs/atomic.h
>
> diff --git a/include/erofs/atomic.h b/include/erofs/atomic.h
> new file mode 100644
> index 0000000..c486491
> --- /dev/null
> +++ b/include/erofs/atomic.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0 */
> +/*
> + * Copyright (C) 2024 Alibaba Cloud
> + */
> +#ifndef __EROFS_ATOMIC_H
> +#define __EROFS_ATOMIC_H
> +
> +/*
> + * Just use GCC/clang built-in functions for now
> + * See:https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
> + */
> +typedef unsigned long erofs_atomic_t;

According to [1] *__atomic_test_and_set *should**only be used for 
operands of type bool or char.

[1] https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

Maybe add

/typedef bool erofs_atomic_bool_t;
/

for this purpose?

Otherwise LGTM and I will include it in my patchset.


Thanks,

Yifan Zhao

> +
> +#define erofs_atomic_read(ptr) ({ \
> +	typeof(*ptr) __n;    \
> +	__atomic_load(ptr, &__n, __ATOMIC_RELAXED); \
> +__n;})
> +
> +#define erofs_atomic_set(ptr, n) do { \
> +	typeof(*ptr) __n = (n);    \
> +	__atomic_store(ptr, &__n, __ATOMIC_RELAXED); \
> +} while(0)
> +
> +#define erofs_atomic_test_and_set(ptr) \
> +	__atomic_test_and_set(ptr, __ATOMIC_RELAXED)
> +
> +#endif
> diff --git a/lib/compressor_deflate.c b/lib/compressor_deflate.c
> index 8629415..60bb2f6 100644
> --- a/lib/compressor_deflate.c
> +++ b/lib/compressor_deflate.c
> @@ -7,6 +7,7 @@
>   #include "erofs/print.h"
>   #include "erofs/config.h"
>   #include "compressor.h"
> +#include "erofs/atomic.h"
>   
>   void *kite_deflate_init(int level, unsigned int dict_size);
>   void kite_deflate_end(void *s);
> @@ -36,6 +37,8 @@ static int compressor_deflate_exit(struct erofs_compress *c)
>   
>   static int compressor_deflate_init(struct erofs_compress *c)
>   {
> +	static erofs_atomic_t __warnonce;
> +
>   	if (c->private_data) {
>   		kite_deflate_end(c->private_data);
>   		c->private_data = NULL;
> @@ -44,9 +47,11 @@ static int compressor_deflate_init(struct erofs_compress *c)
>   	if (IS_ERR_VALUE(c->private_data))
>   		return PTR_ERR(c->private_data);
>   
> -	erofs_warn("EXPERIMENTAL DEFLATE algorithm in use. Use at your own risk!");
> -	erofs_warn("*Carefully* check filesystem data correctness to avoid corruption!");
> -	erofs_warn("Please send a report to<linux-erofs at lists.ozlabs.org>  if something is wrong.");
> +	if (!erofs_atomic_test_and_set(&__warnonce)) {
> +		erofs_warn("EXPERIMENTAL DEFLATE algorithm in use. Use at your own risk!");
> +		erofs_warn("*Carefully* check filesystem data correctness to avoid corruption!");
> +		erofs_warn("Please send a report to<linux-erofs at lists.ozlabs.org>  if something is wrong.");
> +	}
>   	return 0;
>   }
>   
> diff --git a/lib/compressor_libdeflate.c b/lib/compressor_libdeflate.c
> index 62d93f7..f90d720 100644
> --- a/lib/compressor_libdeflate.c
> +++ b/lib/compressor_libdeflate.c
> @@ -4,6 +4,7 @@
>   #include "erofs/config.h"
>   #include <libdeflate.h>
>   #include "compressor.h"
> +#include "erofs/atomic.h"
>   
>   static int libdeflate_compress_destsize(const struct erofs_compress *c,
>   				        const void *src, unsigned int *srcsize,
> @@ -82,12 +83,15 @@ static int compressor_libdeflate_exit(struct erofs_compress *c)
>   
>   static int compressor_libdeflate_init(struct erofs_compress *c)
>   {
> +	static erofs_atomic_t __warnonce;
> +
>   	libdeflate_free_compressor(c->private_data);
>   	c->private_data = libdeflate_alloc_compressor(c->compression_level);
>   	if (!c->private_data)
>   		return -ENOMEM;
>   
> -	erofs_warn("EXPERIMENTAL libdeflate compressor in use. Use at your own risk!");
> +	if (!erofs_atomic_test_and_set(&__warnonce))
> +		erofs_warn("EXPERIMENTAL libdeflate compressor in use. Use at your own risk!");
>   	return 0;
>   }
>   
> diff --git a/lib/compressor_liblzma.c b/lib/compressor_liblzma.c
> index 712f44f..e79717d 100644
> --- a/lib/compressor_liblzma.c
> +++ b/lib/compressor_liblzma.c
> @@ -9,6 +9,7 @@
>   #include "erofs/config.h"
>   #include "erofs/print.h"
>   #include "erofs/internal.h"
> +#include "erofs/atomic.h"
>   #include "compressor.h"
>   
>   struct erofs_liblzma_context {
> @@ -85,6 +86,7 @@ static int erofs_compressor_liblzma_init(struct erofs_compress *c)
>   {
>   	struct erofs_liblzma_context *ctx;
>   	u32 preset;
> +	static erofs_atomic_t __warnonce;
>   
>   	ctx = malloc(sizeof(*ctx));
>   	if (!ctx)
> @@ -103,7 +105,8 @@ static int erofs_compressor_liblzma_init(struct erofs_compress *c)
>   	ctx->opt.dict_size = c->dict_size;
>   
>   	c->private_data = ctx;
> -	erofs_warn("It may take a longer time since MicroLZMA is still single-threaded for now.");
> +	if (!erofs_atomic_test_and_set(&__warnonce))
> +		erofs_warn("It may take a longer time since MicroLZMA is still single-threaded for now.");
>   	return 0;
>   }
>   
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-erofs/attachments/20240228/bc167bb4/attachment.htm>