From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f170.google.com (mail-pg1-f170.google.com [209.85.215.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 71314414DCA for ; Wed, 5 Aug 2026 10:25:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785925531; cv=none; b=Ok8vpjTH2BlAeuOxcAK7YV88WsxYf1Dx8qe5+fhhF/6VGinPdGN4DmVLvpGOueX//Rz0l6Lc8EOA8loAYaWzv/43lMy84QjRPPhvHa2bEpKkmXECmqadMFsBDyW2+dzFZwanqOtw4zKKL5zWqALanbps7Vu6qFhb4mwVln8ipf8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785925531; c=relaxed/simple; bh=FolS/za1KEwulDAC+P0pDLZB9kFc1wK7xoo3Udu+wiA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KMynjGURjRmPGsS6DNIKXoG8NYSsZBH/sH20SE+hg+MxVV4pBXbzGDhqJeXUEfDlv+piCFfiEnNs8wrjnznK2FWNrCT16k2VUN1FGUBtEwr5VC7a3x0K5FV8vYmbTV6xuBfFttZeTmWUdNZYXqf58ZsPtHNFQ+T9n2Gha/dJlyI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=B8ZeBPmj; arc=none smtp.client-ip=209.85.215.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="B8ZeBPmj" Received: by mail-pg1-f170.google.com with SMTP id 41be03b00d2f7-c96b08cdd1cso594293a12.0 for ; Wed, 05 Aug 2026 03:25:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1785925529; x=1786530329; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=g9y18o+SeXXE89bm99BSkZE54Wz40yQEuA61nq1G8kI=; b=B8ZeBPmjULKS27CEoBwJFHEhZEa8pnondeKt04N93tsse7/hMrzS65luK98/YRtwje pEUs3eGs3Zz9A9aeH1enDnvAifjSuYWMDsHcRFaGZdY8YDGWUGA+vrpCbc+bAtxPwzsK 0I/nUgJpFeGaBNypkooSG/WRTTF6LqVpnqImY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785925529; x=1786530329; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=g9y18o+SeXXE89bm99BSkZE54Wz40yQEuA61nq1G8kI=; b=fmoMnk1hQLBCJGhqaYFm/wR2gGvBLi3MKUVfySySfY092O6HTAnSATd6BoHP4qBSeU COGC9/Tam1hwBZZV0jQhOjsNwZTp85cDJbYIELYRbW5IYPiSSezvAziA74oZmnqVkMog sJh/q0dmfU/LxiAMgnaIui8lTduLkgzHofboRO4/r8WklvO3lXVhBSIgOy4gSHbaMPjB gNIxRnW1PCEBAqDo9k7BU1XUC0UabyS+Fc3PC1zIa6fvo4ZN1Stj1y1Yg3p2GEhcofIx IEa3mCB3698Opsmt2v2qKPza0ZvxbDru/drEvSlAF7Wcuo17z35jBEgJRxt1lRq7oXP4 nVRA== X-Forwarded-Encrypted: i=1; AHgh+RrOYEKbFj7agbQ1866W3zKpoYrBhRmx6kzbXzLcyz3oQuH/PzAkujYKO2QuM4C617g01i6cClShvqmxd14=@vger.kernel.org X-Gm-Message-State: AOJu0YxF8ICXlzS3Ro1Zqzgtvne92B+sybdVOu9AT1PrzUac3yNkpfWa jNPgDggGkKQB4JWWKZ323NCgmIC6XE1yUW0mmYUKZetkaPQnJLkgRuSfLu4ysoe3mA== X-Gm-Gg: AR+sD10tMYWQgkjiM6LeM7tx9FOTLsLrokZ5oOEWW551ehVKf0TSqCJVia3n3VTwVvx C7ie3Ek4qZ1nx3h707y7FQ1dRIw+jpaYk7NHwcU4r4s7JzGJK6cm39vGiydhC6ZpRwjejywjGmv HqgIzmr27dusCiBtU/K4djP1z9aFkHI+DJzwZLvKxlu7BPpJx3kpRU0XpZwlm992uWeIGYtdERb zSKcqzQzgVp3PlbOuS6+Vmyg9BAkGYIZOsckYTB7hFbYz7EO2R5xFr2OayGOXyhEo7E0GPk+La7 sTjWubEo9JiuqQvzBvXlAA8RX3QF2XAMB6QUoTUR1U7v57Nyc5CxJMBySAs5mQlPoeiS/HnD12t qPVd4coKBvYZ2kjcWQHCTOBq6TxVGRRNAuga9wdTfdvOTW92nqpOBeot5v+mL2hyJSPXGKflxJA 2yLJ27MghlknnSBqdktQ/vVhaVWSSSB79V6hm0ksme7u6bN9iFXVSrw9CIIk3+fNc4XKR6l5khP 2Zht6bn6HdIp62X4yoraECI58Nf X-Received: by 2002:a05:6a21:2293:b0:3c3:994f:b4f5 with SMTP id adf61e73a8af0-3cb85eefa11mr7619522637.28.1785925528727; Wed, 05 Aug 2026 03:25:28 -0700 (PDT) Received: from google.com ([2a00:79e0:2031:6:8002:2a47:a704:5a58]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-cbe708041basm1245872a12.14.2026.08.05.03.25.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Aug 2026 03:25:27 -0700 (PDT) Date: Wed, 5 Aug 2026 19:25:23 +0900 From: Sergey Senozhatsky To: Barry Song Cc: Sergey Senozhatsky , akpm@linux-foundation.org, bigeasy@linutronix.de, hdanton@sina.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, minchan@kernel.org, ryncsn@gmail.com, yosry.ahmed@linux.dev, surenb@google.com, Dongdong Zhang , Suleiman Souhlal Subject: Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends Message-ID: References: <20260805005545.66112-1-baohua@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Hi Barry, On (26/08/05 15:50), Barry Song wrote: > BTW, I wonder if compression and decompression could use separate > mutexes. That way, a sleepable zs_malloc() in the compression path > would not block decompression, which is the more latency-sensitive > operation. quick and dirty patch. Just curious if this improves anything on your side. We also maybe can have more that num_online_cpus() stream, if we switch to idle streams list instead [1] [1] https://lore.kernel.org/lkml/20250130111105.2861324-3-senozhatsky@chromium.org/ ---- diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 974c4691887e..3c523ea0dc27 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -112,21 +112,28 @@ ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at) return at; } -struct zcomp_strm *zcomp_stream_get(struct zcomp *comp) +struct zcomp_strm *zcomp_stream_get_write(struct zcomp *comp) { for (;;) { - struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream); - - /* - * Inspired by zswap - * - * stream is returned with ->mutex locked which prevents - * cpu_dead() from releasing this stream under us, however - * there is still a race window between raw_cpu_ptr() and - * mutex_lock(), during which we could have been migrated - * from a CPU that has already destroyed its stream. If - * so then unlock and re-try on the current CPU. - */ + struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream_write); + + mutex_lock(&zstrm->lock); + if (likely(zstrm->buffer)) + return zstrm; + mutex_unlock(&zstrm->lock); + } +} + +void zcomp_stream_put_write(struct zcomp_strm *zstrm) +{ + mutex_unlock(&zstrm->lock); +} + +struct zcomp_strm *zcomp_stream_get_read(struct zcomp *comp) +{ + for (;;) { + struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream_read); + mutex_lock(&zstrm->lock); if (likely(zstrm->buffer)) return zstrm; @@ -134,7 +141,7 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp) } } -void zcomp_stream_put(struct zcomp_strm *zstrm) +void zcomp_stream_put_read(struct zcomp_strm *zstrm) { mutex_unlock(&zstrm->lock); } @@ -174,23 +181,39 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm, int zcomp_cpu_up_prepare(unsigned int cpu, struct hlist_node *node) { struct zcomp *comp = hlist_entry(node, struct zcomp, node); - struct zcomp_strm *zstrm = per_cpu_ptr(comp->stream, cpu); + struct zcomp_strm *zstrm_w = per_cpu_ptr(comp->stream_write, cpu); + struct zcomp_strm *zstrm_r = per_cpu_ptr(comp->stream_read, cpu); int ret; - ret = zcomp_strm_init(comp, zstrm); - if (ret) - pr_err("Can't allocate a compression stream\n"); - return ret; + ret = zcomp_strm_init(comp, zstrm_w); + if (ret) { + pr_err("Can't allocate a compression write stream\n"); + return ret; + } + + ret = zcomp_strm_init(comp, zstrm_r); + if (ret) { + pr_err("Can't allocate a compression read stream\n"); + zcomp_strm_free(comp, zstrm_w); + return ret; + } + + return 0; } int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node) { struct zcomp *comp = hlist_entry(node, struct zcomp, node); - struct zcomp_strm *zstrm = per_cpu_ptr(comp->stream, cpu); + struct zcomp_strm *zstrm_w = per_cpu_ptr(comp->stream_write, cpu); + struct zcomp_strm *zstrm_r = per_cpu_ptr(comp->stream_read, cpu); - mutex_lock(&zstrm->lock); - zcomp_strm_free(comp, zstrm); - mutex_unlock(&zstrm->lock); + mutex_lock(&zstrm_w->lock); + zcomp_strm_free(comp, zstrm_w); + mutex_unlock(&zstrm_w->lock); + + mutex_lock(&zstrm_r->lock); + zcomp_strm_free(comp, zstrm_r); + mutex_unlock(&zstrm_r->lock); return 0; } @@ -198,17 +221,25 @@ static int zcomp_init(struct zcomp *comp, struct zcomp_params *params) { int ret, cpu; - comp->stream = alloc_percpu(struct zcomp_strm); - if (!comp->stream) + comp->stream_write = alloc_percpu(struct zcomp_strm); + if (!comp->stream_write) return -ENOMEM; + comp->stream_read = alloc_percpu(struct zcomp_strm); + if (!comp->stream_read) { + free_percpu(comp->stream_write); + return -ENOMEM; + } + comp->params = params; ret = comp->ops->setup_params(comp->params); if (ret) goto cleanup; - for_each_possible_cpu(cpu) - mutex_init(&per_cpu_ptr(comp->stream, cpu)->lock); + for_each_possible_cpu(cpu) { + mutex_init(&per_cpu_ptr(comp->stream_write, cpu)->lock); + mutex_init(&per_cpu_ptr(comp->stream_read, cpu)->lock); + } ret = cpuhp_state_add_instance(CPUHP_ZCOMP_PREPARE, &comp->node); if (ret < 0) @@ -218,7 +249,8 @@ static int zcomp_init(struct zcomp *comp, struct zcomp_params *params) cleanup: comp->ops->release_params(comp->params); - free_percpu(comp->stream); + free_percpu(comp->stream_read); + free_percpu(comp->stream_write); return ret; } @@ -226,7 +258,8 @@ void zcomp_destroy(struct zcomp *comp) { cpuhp_state_remove_instance(CPUHP_ZCOMP_PREPARE, &comp->node); comp->ops->release_params(comp->params); - free_percpu(comp->stream); + free_percpu(comp->stream_read); + free_percpu(comp->stream_write); kfree(comp); } diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index 81a0f3f6ff48..fd919571d8b7 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -71,7 +71,8 @@ struct zcomp_ops { /* dynamic per-device compression frontend */ struct zcomp { - struct zcomp_strm __percpu *stream; + struct zcomp_strm __percpu *stream_write; + struct zcomp_strm __percpu *stream_read; const struct zcomp_ops *ops; struct zcomp_params *params; struct hlist_node node; @@ -85,8 +86,11 @@ const char *zcomp_lookup_backend_name(const char *comp); struct zcomp *zcomp_create(const char *alg, struct zcomp_params *params); void zcomp_destroy(struct zcomp *comp); -struct zcomp_strm *zcomp_stream_get(struct zcomp *comp); -void zcomp_stream_put(struct zcomp_strm *zstrm); +struct zcomp_strm *zcomp_stream_get_write(struct zcomp *comp); +void zcomp_stream_put_write(struct zcomp_strm *zstrm); + +struct zcomp_strm *zcomp_stream_get_read(struct zcomp *comp); +void zcomp_stream_put_read(struct zcomp_strm *zstrm); int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, const void *src, unsigned int *dst_len); diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 3b9dfcae9317..6a564a5e0041 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1358,14 +1358,14 @@ static int decompress_bdev_page(struct zram *zram, struct page *page, u32 index) size = get_slot_size(zram, index); prio = get_slot_comp_priority(zram, index); - zstrm = zcomp_stream_get(zram->comps[prio]); + zstrm = zcomp_stream_get_read(zram->comps[prio]); src = kmap_local_page(page); ret = zcomp_decompress(zram->comps[prio], zstrm, src, size, zstrm->local_copy); if (!ret) copy_page(src, zstrm->local_copy); kunmap_local(src); - zcomp_stream_put(zstrm); + zcomp_stream_put_read(zstrm); slot_unlock(zram, index); return ret; @@ -2101,14 +2101,14 @@ static int read_compressed_page(struct zram *zram, struct page *page, u32 index) size = get_slot_size(zram, index); prio = get_slot_comp_priority(zram, index); - zstrm = zcomp_stream_get(zram->comps[prio]); + zstrm = zcomp_stream_get_read(zram->comps[prio]); src = zs_obj_read_begin(zram->mem_pool, handle, size, zstrm->local_copy); dst = kmap_local_page(page); ret = zcomp_decompress(zram->comps[prio], zstrm, src, size, dst); kunmap_local(dst); zs_obj_read_end(zram->mem_pool, handle, size, src); - zcomp_stream_put(zstrm); + zcomp_stream_put_read(zstrm); return ret; } @@ -2129,12 +2129,12 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page, u32 index) * case if object spans two physical pages. No decompression * takes place here, as we read raw compressed data. */ - zstrm = zcomp_stream_get(zram->comps[ZRAM_PRIMARY_COMP]); + zstrm = zcomp_stream_get_read(zram->comps[ZRAM_PRIMARY_COMP]); src = zs_obj_read_begin(zram->mem_pool, handle, size, zstrm->local_copy); memcpy_to_page(page, 0, src, size); zs_obj_read_end(zram->mem_pool, handle, size, src); - zcomp_stream_put(zstrm); + zcomp_stream_put_read(zstrm); memzero_page(page, size, PAGE_SIZE - size); @@ -2285,20 +2285,20 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index) if (same_filled) return write_same_filled_page(zram, element, index); - zstrm = zcomp_stream_get(zram->comps[ZRAM_PRIMARY_COMP]); + zstrm = zcomp_stream_get_write(zram->comps[ZRAM_PRIMARY_COMP]); mem = kmap_local_page(page); ret = zcomp_compress(zram->comps[ZRAM_PRIMARY_COMP], zstrm, mem, &comp_len); kunmap_local(mem); if (unlikely(ret)) { - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); pr_err("Compression failed! err=%d\n", ret); return ret; } if (comp_len >= huge_class_size) { - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); return write_incompressible_page(zram, page, index); } @@ -2306,18 +2306,18 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index) GFP_NOIO | __GFP_NOWARN | __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page)); if (IS_ERR_VALUE(handle)) { - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); return PTR_ERR((void *)handle); } if (!zram_can_store_page(zram)) { - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); zs_free(zram->mem_pool, handle); return -ENOMEM; } zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len); - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); slot_lock(zram, index); slot_free(zram, index); @@ -2457,7 +2457,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page, */ clear_slot_flag(zram, index, ZRAM_IDLE); - zstrm = zcomp_stream_get(zram->comps[prio]); + zstrm = zcomp_stream_get_write(zram->comps[prio]); src = kmap_local_page(page); ret = zcomp_compress(zram->comps[prio], zstrm, src, &comp_len_new); kunmap_local(src); @@ -2472,7 +2472,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page, *num_recomp_pages -= 1; if (ret) { - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); return ret; } @@ -2481,7 +2481,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page, if (class_index_new >= class_index_old || (threshold && comp_len_new >= threshold)) { - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); /* * Secondary algorithms failed to re-compress the page @@ -2510,12 +2510,12 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page, __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page)); if (IS_ERR_VALUE(handle_new)) { - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); return PTR_ERR((void *)handle_new); } zs_obj_write(zram->mem_pool, handle_new, zstrm->buffer, comp_len_new); - zcomp_stream_put(zstrm); + zcomp_stream_put_write(zstrm); slot_free(zram, index); set_slot_handle(zram, index, handle_new);