Linux-EROFS Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Gao Xiang <hsiangkao@linux.alibaba.com>
To: Nithurshen <nithurshen.dev@gmail.com>, linux-erofs@lists.ozlabs.org
Cc: xiang@kernel.org
Subject: Re: [PATCH 1/2] fsck.erofs: add multi-threaded decompression
Date: Mon, 22 Jun 2026 10:08:59 +0800	[thread overview]
Message-ID: <6d8549f5-552d-4a81-b77c-72d63b4c8728@linux.alibaba.com> (raw)
In-Reply-To: <20260621120121.73114-2-nithurshen.dev@gmail.com>

Hi Nithurshen,

On 2026/6/21 20:01, Nithurshen wrote:
> Currently, fsck.erofs extracts files synchronously. When decompressing
> heavily packed images, the main thread spends the majority of its time
> blocked on a combination of synchronous vfs_write() syscalls and
> decompression routines, bottlenecking overall extraction speed.
> 
> This patch introduces a scalable, multi-threaded decompression framework
> using the existing erofs_workqueue infrastructure to decouple compute
> from the main thread's I/O.
> 
> To prevent massive scheduling overhead (futex contention) where worker
> threads spend more CPU time waking up than actually decompressing small
> clusters, this implementation introduces a batching context. Because
> different compression algorithms exhibit vastly different scheduling
> thresholds, the batch size is algorithm-aware:
> - Fast algorithms like LZ4 utilize a larger batch limit (up to 32
>    pclusters) to effectively hide synchronization overhead.
> - Compute-heavy algorithms like LZMA or ZSTD trigger at a lower
>    threshold (8 pclusters) to prevent memory bloat and thread starvation.
> 
> Key details of this implementation:
> - The worker pool is dynamically sized based on available system CPUs.
> - Decompression tasks take strict ownership of the raw and output
>    buffers (safely tracking memory via a `free_out` flag) to prevent
>    data races and memory leaks.
> - Output buffers are explicitly zero-initialized via calloc() to
>    prevent trailing garbage bytes from leaking into extracted files.
> - Tail-end packed fragments are processed synchronously by the main
>    thread, as their minimal overhead does not benefit from asynchronous
>    offloading.
> 
> Signed-off-by: Nithurshen <nithurshen.dev@gmail.com>
> ---
>   fsck/main.c              | 150 ++++++++++++---------------
>   include/erofs/cond.h     |  31 ++++++
>   include/erofs/internal.h |  20 +++-
>   include/erofs/lock.h     |   3 +
>   lib/data.c               | 216 +++++++++++++++++++++++++++++----------
>   5 files changed, 277 insertions(+), 143 deletions(-)
>   create mode 100644 include/erofs/cond.h
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 16cc627..ffe7e29 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -8,14 +8,18 @@
>   #include <time.h>
>   #include <utime.h>
>   #include <unistd.h>
> +#include "erofs/lock.h"
>   #include <sys/stat.h>
>   #include "erofs/print.h"
>   #include "erofs/decompress.h"
>   #include "erofs/dir.h"
>   #include "erofs/xattr.h"
> +#include "erofs/workqueue.h"
>   #include "../lib/compressor.h"
>   #include "../lib/liberofs_compress.h"
>   
> +extern struct erofs_workqueue erofs_wq;
> +
>   static int erofsfsck_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
>   
>   struct erofsfsck_dirstack {
> @@ -505,44 +509,36 @@ out:
>   
>   static int erofs_verify_inode_data(struct erofs_inode *inode, int outfd)
>   {
> -	struct erofs_map_blocks map = {
> -		.buf = __EROFS_BUF_INITIALIZER,
> -	};
> +	struct erofs_map_blocks map = { .buf = __EROFS_BUF_INITIALIZER };

If it's unrelated to the change, let's not touch it.

>   	bool needdecode = fsckcfg.check_decomp && !erofs_is_packed_inode(inode);
>   	int ret = 0;
> -	bool compressed;
> +	bool compressed = erofs_inode_is_data_compressed(inode->datalayout);
>   	erofs_off_t pos = 0;
>   	u64 pchunk_len = 0;
> -	unsigned int raw_size = 0, buffer_size = 0;
> -	char *raw = NULL, *buffer = NULL;
>   
> -	erofs_dbg("verify data chunk of nid(%llu): type(%d)",
> -		  inode->nid | 0ULL, inode->datalayout);
> +	struct z_erofs_read_ctx ctx = {

maybe rename it as z_erofs_mt_read_ctx since it relates to multi-threading.

> +		.pending_tasks = 0,
> +		.final_err = 0,
> +		.outfd = outfd,
> +		.free_out = true,
> +		.current_task = NULL
> +	};
> +	erofs_mutex_init(&ctx.lock);
> +	erofs_cond_init(&ctx.cond);
>   
> -	compressed = erofs_inode_is_data_compressed(inode->datalayout);
> -	while (pos < inode->i_size) {
> -		unsigned int alloc_rawsize;
> +	erofs_dbg("verify data chunk of nid(%llu): type(%d)", inode->nid | 0ULL, inode->datalayout);

Can we move this to the original position? Unnecessary diff.

>   
> +	while (pos < inode->i_size) {
>   		map.m_la = pos;
>   		ret = erofs_map_blocks(inode, &map, EROFS_GET_BLOCKS_FIEMAP);
> -		if (ret)
> -			goto out;
> -
> -		if (!compressed && map.m_llen != map.m_plen) {
> -			erofs_err("broken chunk length m_la %" PRIu64 " m_llen %" PRIu64 " m_plen %" PRIu64,
> -				  map.m_la, map.m_llen, map.m_plen);
> -			ret = -EFSCORRUPTED;
> -			goto out;
> -		}
> +		if (ret) goto out;

The code style is still wrong here.

>   
> -		/* the last lcluster can be divided into 3 parts */
>   		if (map.m_la + map.m_llen > inode->i_size)
>   			map.m_llen = inode->i_size - map.m_la;
>   
>   		pchunk_len += map.m_plen;
>   		pos += map.m_llen;
>   
> -		/* should skip decomp? */
>   		if (map.m_la >= inode->i_size || !needdecode)
>   			continue;
>   
> @@ -555,85 +551,53 @@ static int erofs_verify_inode_data(struct erofs_inode *inode, int outfd)
>   			continue;
>   		}
>   
> -		if (map.m_plen > Z_EROFS_PCLUSTER_MAX_SIZE) {
> -			if (compressed && !(map.m_flags & __EROFS_MAP_FRAGMENT)) {
> -				erofs_err("invalid pcluster size %" PRIu64 " @ offset %" PRIu64 " of nid %" PRIu64,
> -					  map.m_plen, map.m_la,
> -					  inode->nid | 0ULL);
> -				ret = -EFSCORRUPTED;
> -				goto out;
> -			}
> -			alloc_rawsize = Z_EROFS_PCLUSTER_MAX_SIZE;
> -		} else {
> -			alloc_rawsize = map.m_plen;
> -		}
> -
> -		if (alloc_rawsize > raw_size) {
> -			char *newraw = realloc(raw, alloc_rawsize);
> -
> -			if (!newraw) {
> +		if (compressed) {
> +			char *raw = malloc(map.m_plen);
> +			size_t buffer_size = map.m_llen > erofs_blksiz(inode->sbi) ? map.m_llen : erofs_blksiz(inode->sbi);

Please avoid overly long lines (< 80 chars recommended, but should
less than 100 chars)

> +			char *buffer = calloc(1, buffer_size);

			I hope it could be:

			char *raw, *buffers;

			raw = malloc(map.m_plen);
			buffer = calloc(1, buffer_size);

> +			
> +			if (!raw || !buffer) {
> +				free(raw); free(buffer);
>   				ret = -ENOMEM;
>   				goto out;
>   			}
> -			raw = newraw;
> -			raw_size = alloc_rawsize;
> -		}
>   
> -		if (compressed) {
> -			if (map.m_llen > buffer_size) {
> -				char *newbuffer;
> -
> -				buffer_size = map.m_llen;
> -				newbuffer = realloc(buffer, buffer_size);
> -				if (!newbuffer) {
> -					ret = -ENOMEM;
> -					goto out;
> -				}
> -				buffer = newbuffer;
> -			}
> -			ret = z_erofs_read_one_data(inode, &map, raw, buffer,
> -						    0, map.m_llen, false);
> -			if (ret)
> +			ret = z_erofs_read_one_data(inode, &map, raw, buffer, 0, map.m_llen, false, map.m_la, &ctx);

Same code styling issue here.

> +			if (ret) {
> +				/* DO NOT free(raw) or free(buffer) here. z_erofs_read_one_data took ownership! */
>   				goto out;
> -
> -			if (outfd >= 0 && write(outfd, buffer, map.m_llen) < 0)
> -				goto fail_eio;
> +			}
>   		} else {
> -			u64 p = 0;
> -
> -			do {
> -				u64 count = min_t(u64, alloc_rawsize,
> -						  map.m_llen);
> -
> -				ret = erofs_read_one_data(inode, &map, raw, p, count);
> -				if (ret)
> -					goto out;
> -
> -				if (outfd >= 0 && write(outfd, raw, count) < 0)
> -					goto fail_eio;
> -				map.m_llen -= count;
> -				p += count;
> -			} while (map.m_llen);
> +			char *raw = calloc(1, map.m_llen);
> +			ret = erofs_read_one_data(inode, &map, raw, 0, map.m_llen);
> +			if (ret >= 0 && outfd >= 0)
> +				pwrite(outfd, raw, map.m_llen, map.m_la);
> +			free(raw);
> +			if (ret) goto out;
>   		}
>   	}
> +	z_erofs_read_ctx_enqueue(&ctx);

Maybe just call it as `z_erofs_mt_read_enqueue(&ctx);`

> +
> +out:
> +	erofs_mutex_lock(&ctx.lock);
> +	while (ctx.pending_tasks > 0)
> +		erofs_cond_wait(&ctx.cond, &ctx.lock);
> +	if (ctx.final_err < 0 && ret >= 0)
> +		ret = ctx.final_err;
> +	erofs_mutex_unlock(&ctx.lock);
>   
>   	if (fsckcfg.print_comp_ratio) {
>   		if (!erofs_is_packed_inode(inode))
>   			fsckcfg.logical_blocks += BLK_ROUND_UP(inode->sbi, inode->i_size);
>   		fsckcfg.physical_blocks += BLK_ROUND_UP(inode->sbi, pchunk_len);
>   	}
> -out:
> -	if (raw)
> -		free(raw);
> -	if (buffer)
> -		free(buffer);
> -	return ret < 0 ? ret : 0;
>   
> -fail_eio:
> -	erofs_err("I/O error occurred when verifying data chunk @ nid %llu",
> -		  inode->nid | 0ULL);
> -	ret = -EIO;
> -	goto out;
> +	if (outfd >= 0 && ret >= 0)
> +		ftruncate(outfd, inode->i_size);
> +	
> +	erofs_mutex_destroy(&ctx.lock);
> +	erofs_cond_destroy(&ctx.cond);
> +	return ret < 0 ? ret : 0;
>   }
>   
>   static inline int erofs_extract_dir(struct erofs_inode *inode)
> @@ -1043,10 +1007,21 @@ int erofsfsck_fuzz_one(int argc, char *argv[])
>   int main(int argc, char *argv[])
>   #endif
>   {
> +
>   	int err;
> +#ifdef EROFS_MT_ENABLED
> +	int workers;
> +#endif
>   
>   	erofs_init_configure();
>   
> +#ifdef EROFS_MT_ENABLED
> +	workers = erofs_get_available_processors();
> +	if (workers < 1)
> +		workers = 1;
> +	erofs_alloc_workqueue(&erofs_wq, workers, 256, NULL, NULL);
> +#endif

It shouldn't be worked as this, we should have a helper
in liberofs to wrap up the initialization.

And erofs_wq naming is ambigious.

> +
>   	fsckcfg.physical_blocks = 0;
>   	fsckcfg.logical_blocks = 0;
>   	fsckcfg.extract_path = NULL;
> @@ -1181,6 +1156,9 @@ exit_dev_close:
>   exit:
>   	erofs_blob_closeall(&g_sbi);
>   	erofs_exit_configure();
> +#ifdef EROFS_MT_ENABLED
> +	erofs_destroy_workqueue(&erofs_wq);
> +#endif

Same here.

>   	return err ? 1 : 0;
>   }
>   
> diff --git a/include/erofs/cond.h b/include/erofs/cond.h
> new file mode 100644
> index 0000000..90ec838
> --- /dev/null
> +++ b/include/erofs/cond.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0 */
> +#ifndef __EROFS_COND_H
> +#define __EROFS_COND_H
> +
> +#include "lock.h"
> +
> +#if defined(HAVE_PTHREAD_H) && defined(EROFS_MT_ENABLED)
> +#include <pthread.h>
> +
> +typedef pthread_cond_t erofs_cond_t;
> +
> +static inline void erofs_cond_init(erofs_cond_t *cond)
> +{
> +	pthread_cond_init(cond, NULL);
> +}
> +#define erofs_cond_wait		pthread_cond_wait
> +#define erofs_cond_signal	pthread_cond_signal
> +#define erofs_cond_broadcast	pthread_cond_broadcast
> +#define erofs_cond_destroy	pthread_cond_destroy
> +
> +#else
> +typedef struct {} erofs_cond_t;
> +
> +static inline void erofs_cond_init(erofs_cond_t *cond) {}
> +static inline int erofs_cond_wait(erofs_cond_t *cond, erofs_mutex_t *mutex) { return 0; }
> +static inline int erofs_cond_signal(erofs_cond_t *cond) { return 0; }
> +static inline int erofs_cond_broadcast(erofs_cond_t *cond) { return 0; }
> +static inline int erofs_cond_destroy(erofs_cond_t *cond) { return 0; }
> +#endif
> +
> +#endif
> \ No newline at end of file
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index 671880f..94f14da 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -25,6 +25,8 @@ typedef unsigned short umode_t;
>   #ifdef HAVE_PTHREAD_H
>   #include <pthread.h>
>   #endif
> +#include <erofs/lock.h>
> +#include "erofs/cond.h"
>   #include <stdlib.h>
>   #include <string.h>
>   #include "atomic.h"
> @@ -62,6 +64,7 @@ struct erofs_buf {
>   #define erofs_pos(sbi, nr)      ((erofs_off_t)(nr) << (sbi)->blkszbits)
>   #define BLK_ROUND_UP(sbi, addr)	\
>   	(roundup(addr, erofs_blksiz(sbi)) >> (sbi)->blkszbits)
> +#define Z_EROFS_PCLUSTER_MAX_BATCH_SIZE 32
>   
>   struct erofs_buffer_head;
>   struct erofs_bufmgr;
> @@ -442,6 +445,20 @@ struct z_erofs_paramset {
>   	char *extraopts;
>   };
>   
> +struct z_erofs_decompress_task;
> +
> +struct z_erofs_read_ctx {
> +	erofs_mutex_t lock;
> +	erofs_cond_t cond;
> +	int pending_tasks;
> +	int final_err;
> +	int outfd;
> +	bool free_out;
> +	struct z_erofs_decompress_task *current_task;
> +};

Is it necessary to expose the internal structure fields
to users?

> +
> +void z_erofs_read_ctx_enqueue(struct z_erofs_read_ctx *ctx);

z_erofs_mt_read_enqueue

> +
>   int liberofs_global_init(void);
>   void liberofs_global_exit(void);
>   
> @@ -478,7 +495,8 @@ int erofs_read_one_data(struct erofs_inode *inode, struct erofs_map_blocks *map,
>   			char *buffer, u64 offset, size_t len);
>   int z_erofs_read_one_data(struct erofs_inode *inode,
>   			struct erofs_map_blocks *map, char *raw, char *buffer,
> -			erofs_off_t skip, erofs_off_t length, bool trimmed);
> +			erofs_off_t skip, erofs_off_t length, bool trimmed,
> +			erofs_off_t out_offset, struct z_erofs_read_ctx *ctx);
>   void *erofs_read_metadata(struct erofs_sb_info *sbi, erofs_nid_t nid,
>   			  erofs_off_t *offset, int *lengthp);
>   int z_erofs_parse_cfgs(struct erofs_sb_info *sbi, struct erofs_super_block *dsb);
> diff --git a/include/erofs/lock.h b/include/erofs/lock.h
> index c6e3093..2e79d52 100644
> --- a/include/erofs/lock.h
> +++ b/include/erofs/lock.h
> @@ -15,6 +15,7 @@ static inline void erofs_mutex_init(erofs_mutex_t *lock)
>   }
>   #define erofs_mutex_lock	pthread_mutex_lock
>   #define erofs_mutex_unlock	pthread_mutex_unlock
> +#define erofs_mutex_destroy	pthread_mutex_destroy
>   
>   #define EROFS_DEFINE_MUTEX(lock)	\
>   	erofs_mutex_t lock = PTHREAD_MUTEX_INITIALIZER
> @@ -29,12 +30,14 @@ static inline void erofs_init_rwsem(erofs_rwsem_t *lock)
>   #define erofs_down_write	pthread_rwlock_wrlock
>   #define erofs_up_read		pthread_rwlock_unlock
>   #define erofs_up_write		pthread_rwlock_unlock
> +
>   #else
>   typedef struct {} erofs_mutex_t;
>   
>   static inline void erofs_mutex_init(erofs_mutex_t *lock) {}
>   static inline void erofs_mutex_lock(erofs_mutex_t *lock) {}
>   static inline void erofs_mutex_unlock(erofs_mutex_t *lock) {}
> +static inline void erofs_mutex_destroy(erofs_mutex_t *lock) {}
>   
>   #define EROFS_DEFINE_MUTEX(lock)	\
>   	erofs_mutex_t lock = {}
> diff --git a/lib/data.c b/lib/data.c
> index 6fd1389..e9d2218 100644
> --- a/lib/data.c
> +++ b/lib/data.c
> @@ -9,6 +9,73 @@
>   #include "erofs/trace.h"
>   #include "erofs/decompress.h"
>   #include "liberofs_fragments.h"
> +#include "erofs/workqueue.h"
> +#include "erofs/lock.h"
> +
> +struct erofs_workqueue erofs_wq;
> +
> +struct z_erofs_decompress_item {
> +	struct z_erofs_decompress_req req;
> +	char *raw_buf;
> +	char *out_buf;
> +	erofs_off_t out_offset;
> +	unsigned int out_length;
> +};
> +
> +struct z_erofs_decompress_task {
> +	struct erofs_work work;
> +	struct z_erofs_read_ctx *ctx;
> +	struct z_erofs_decompress_item items[Z_EROFS_PCLUSTER_MAX_BATCH_SIZE];
> +	unsigned int nr_reqs;
> +};
> +
> +static void z_erofs_decompress_worker(struct erofs_work *work, void *tlsp)
> +{
> +	struct z_erofs_decompress_task *task = (struct z_erofs_decompress_task *)work;
> +	struct z_erofs_read_ctx *ctx = task->ctx;
> +	int i, ret = 0, first_err = 0;
> +
> +	for (i = 0; i < task->nr_reqs; ++i) {
> +		struct z_erofs_decompress_item *item = &task->items[i];
> +		ret = z_erofs_decompress(&item->req);
> +
> +		if (ret >= 0 && ctx && ctx->outfd >= 0) {
> +			if (pwrite(ctx->outfd, item->out_buf,
> +				   item->out_length, item->out_offset) < 0)
> +				ret = -errno;
> +		}
> +
> +		if (ret < 0 && first_err == 0)

We don't use first_err == 0 style, it should be !first_err.

> +			first_err = ret;
> +
> +		free(item->raw_buf);
> +		if (ctx && ctx->free_out)
> +			free(item->out_buf);
> +	}
> +
> +	if (ctx) {
> +		erofs_mutex_lock(&ctx->lock);
> +		if (first_err < 0 && ctx->final_err == 0)
> +			ctx->final_err = first_err;
> +		ctx->pending_tasks--;
> +		if (ctx->pending_tasks == 0)

Same here.

> +			erofs_cond_signal(&ctx->cond);
> +		erofs_mutex_unlock(&ctx->lock);
> +	}
> +	free(task);
> +}
> +
> +void z_erofs_read_ctx_enqueue(struct z_erofs_read_ctx *ctx)
> +{
> +	if (ctx && ctx->current_task) {
> +#ifdef EROFS_MT_ENABLED
> +		erofs_queue_work(&erofs_wq, &ctx->current_task->work);
> +#else
> +		z_erofs_decompress_worker(&ctx->current_task->work, NULL);
> +#endif
> +		ctx->current_task = NULL;
> +	}
> +}
>   
>   void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
>   {
> @@ -277,7 +344,8 @@ static int erofs_read_raw_data(struct erofs_inode *inode, char *buffer,
>   
>   int z_erofs_read_one_data(struct erofs_inode *inode,
>   			struct erofs_map_blocks *map, char *raw, char *buffer,
> -			erofs_off_t skip, erofs_off_t length, bool trimmed)
> +			erofs_off_t skip, erofs_off_t length, bool trimmed,
> +			erofs_off_t out_offset, struct z_erofs_read_ctx *ctx)
>   {
>   	struct erofs_sb_info *sbi = inode->sbi;
>   	struct erofs_map_dev mdev;
> @@ -285,77 +353,107 @@ int z_erofs_read_one_data(struct erofs_inode *inode,
>   
>   	if (map->m_flags & __EROFS_MAP_FRAGMENT) {
>   		if (__erofs_unlikely(inode->nid == sbi->packed_nid)) {
> -			erofs_err("fragment should not exist in the packed inode %llu",
> -				  sbi->packed_nid | 0ULL);
> -			return -EFSCORRUPTED;
> +			ret = -EFSCORRUPTED;
> +			goto err_out;
> +		}
> +		ret = erofs_packedfile_read(sbi, buffer, length - skip,
> +					   inode->fragmentoff + skip);
> +		
> +		if (ret >= 0 && ctx && ctx->outfd >= 0) {
> +			if (pwrite(ctx->outfd, buffer, length - skip, out_offset) < 0)
> +				ret = -errno;
>   		}
> -		return erofs_packedfile_read(sbi, buffer, length - skip,
> -				   inode->fragmentoff + skip);
> +		goto err_out;
>   	}
>   
> -	/* no device id here, thus it will always succeed */
> -	mdev = (struct erofs_map_dev) {
> -		.m_pa = map->m_pa,
> -	};
> +	mdev = (struct erofs_map_dev) { .m_pa = map->m_pa };

Please don't change unrelated code.

>   	ret = erofs_map_dev(sbi, &mdev);
> -	if (ret) {
> -		DBG_BUGON(1);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_out;
>   
>   	ret = erofs_dev_read(sbi, mdev.m_deviceid, raw, mdev.m_pa, map->m_plen);
>   	if (ret < 0)
> -		return ret;
> +		goto err_out;
> +	struct z_erofs_decompress_task *task = ctx->current_task;
> +	if (!task) {
> +		task = calloc(1, sizeof(*task));
> +		task->ctx = ctx;
> +		task->work.fn = z_erofs_decompress_worker;
> +		ctx->current_task = task;
> +
> +		erofs_mutex_lock(&ctx->lock);
> +		ctx->pending_tasks++;
> +		erofs_mutex_unlock(&ctx->lock);
> +	}
>   
> -	ret = z_erofs_decompress(&(struct z_erofs_decompress_req) {
> -			.sbi = sbi,
> -			.in = raw,
> -			.out = buffer,
> -			.decodedskip = skip,
> -			.interlaced_offset =
> -				map->m_algorithmformat == Z_EROFS_COMPRESSION_INTERLACED ?
> -					erofs_blkoff(sbi, map->m_la) : 0,
> -			.inputsize = map->m_plen,
> -			.decodedlength = length,
> -			.alg = map->m_algorithmformat,
> -			.partial_decoding = trimmed ? true :
> -				!(map->m_flags & EROFS_MAP_FULL_MAPPED) ||
> -					(map->m_flags & EROFS_MAP_PARTIAL_REF),
> -			 });
> -	if (ret < 0)
> -		return ret;
> +	int idx = task->nr_reqs++;
> +	struct z_erofs_decompress_item *item = &task->items[idx];

Definitions should be at the beginning of the code blocks.

> +
> +	item->req = (struct z_erofs_decompress_req) {
> +		.sbi = sbi,
> +		.in = raw,
> +		.out = buffer,
> +		.decodedskip = skip,
> +		.interlaced_offset =
> +			map->m_algorithmformat == Z_EROFS_COMPRESSION_INTERLACED ?
> +				erofs_blkoff(sbi, map->m_la) : 0,
> +		.inputsize = map->m_plen,
> +		.decodedlength = length,
> +		.alg = map->m_algorithmformat,
> +		.partial_decoding = trimmed ? true :
> +			!(map->m_flags & EROFS_MAP_FULL_MAPPED) ||
> +				(map->m_flags & EROFS_MAP_PARTIAL_REF),
> +	};
> +	item->raw_buf = raw;
> +	item->out_buf = buffer;
> +	item->out_offset = out_offset;
> +	item->out_length = length;
> +
> +	int batch_limit = (map->m_algorithmformat == Z_EROFS_COMPRESSION_LZ4) ?
> +						Z_EROFS_PCLUSTER_MAX_BATCH_SIZE : 8;

Same here.

> +
> +	if (task->nr_reqs >= batch_limit) {
> +		z_erofs_read_ctx_enqueue(ctx);
> +	}
>   	return 0;
> +
> +err_out:
> +	if (ctx && ctx->free_out) free(buffer);

Wrong style, please fix them all.

> +	free(raw);
> +	return ret;
>   }
>   
>   static int z_erofs_read_data(struct erofs_inode *inode, char *buffer,
> -			     erofs_off_t size, erofs_off_t offset)
> +				 erofs_off_t size, erofs_off_t offset)
>   {
>   	erofs_off_t end, length, skip;
>   	struct erofs_map_blocks map = {
>   		.buf = __EROFS_BUF_INITIALIZER,
>   	};
>   	bool trimmed;
> -	unsigned int bufsize = 0;
> -	char *raw = NULL;
>   	int ret = 0;
>   
> +	struct z_erofs_read_ctx ctx = {
> +		.pending_tasks = 0,
> +		.final_err = 0,
> +		.outfd = -1,
> +		.free_out = false,
> +		.current_task = NULL
> +	};
> +	erofs_mutex_init(&ctx.lock);
> +	erofs_cond_init(&ctx.cond);
> +
>   	end = offset + size;
>   	while (end > offset) {
>   		map.m_la = end - 1;
>   
>   		ret = z_erofs_map_blocks_iter(inode, &map, 0);
> -		if (ret)
> -			break;
> +		if (ret) break;

Wrong style again.

Thanks,
Gao Xiang


  reply	other threads:[~2026-06-22  2:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-21 12:01 [PATCH 0/2] fsck.erofs: implement multi-threaded extraction Nithurshen
2026-06-21 12:01 ` [PATCH 1/2] fsck.erofs: add multi-threaded decompression Nithurshen
2026-06-22  2:08   ` Gao Xiang [this message]
2026-06-29  9:55   ` [PATCH 1/2 v2] " Nithurshen
2026-07-05 14:00     ` Gao Xiang
2026-07-06  6:05   ` [PATCH 1/2 v3] " Nithurshen
2026-07-06  6:10     ` Nithurshen
2026-07-08  2:31       ` Gao Xiang
2026-07-13  7:02     ` [PATCH 1/2 v4] " Nithurshen
2026-06-21 12:01 ` [PATCH 2/2] fsck.erofs: implement concurrent directory traversal Nithurshen
2026-07-05 14:05   ` Gao Xiang
2026-07-14  1:10   ` [PATCH v5] fsck.erofs: add multi-threaded decompression Nithurshen
2026-07-14  1:18     ` Nithurshen Karthikeyan
2026-07-26 11:01     ` [PATCH v6] " Nithurshen
2026-07-26 11:02       ` Nithurshen
2026-07-24 14:35   ` [PATCH v2 2/2] fsck.erofs: implement concurrent directory traversal Nithurshen
2026-07-24 14:47     ` Nithurshen
2026-07-24 15:28       ` Gao Xiang
2026-07-24 15:32         ` Nithurshen Karthikeyan
2026-07-24 15:45           ` Gao Xiang
2026-07-24 15:53     ` Gao Xiang
2026-07-25  1:26   ` [PATCH v3 " Nithurshen

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=6d8549f5-552d-4a81-b77c-72d63b4c8728@linux.alibaba.com \
    --to=hsiangkao@linux.alibaba.com \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=nithurshen.dev@gmail.com \
    --cc=xiang@kernel.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