| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <sciqwitrhiosfwo3mrqy5ybf3u65m7vghcet7mnefxyh7uwl2m@viciqkpuoozn>
Date: Sun, 15 Jun 2025 13:51:03 +0800
From: Coly Li <colyli@...nel.org>
To: Kuan-Wei Chiu <visitorckw@...il.com>
Cc: akpm@...ux-foundation.org, kent.overstreet@...ux.dev,
robertpang@...gle.com, linux-kernel@...r.kernel.org, linux-bcache@...r.kernel.org,
jserv@...s.ncku.edu.tw, stable@...r.kernel.org
Subject: Re: [PATCH v2 1/3] Revert "bcache: update min_heap_callbacks to use
default builtin swap"
On Sun, Jun 15, 2025 at 04:23:51AM +0800, Kuan-Wei Chiu wrote:
> This reverts commit 3d8a9a1c35227c3f1b0bd132c9f0a80dbda07b65.
>
> Although removing the custom swap function simplified the code, this
> change is part of a broader migration to the generic min_heap API that
> introduced significant performance regressions in bcache.
>
> As reported by Robert, bcache now suffers from latency spikes, with
> P100 (max) latency increasing from 600 ms to 2.4 seconds every 5
> minutes. These regressions degrade bcache's effectiveness as a
> low-latency cache layer and lead to frequent timeouts and application
> stalls in production environments.
>
> This revert is part of a series of changes to restore previous
> performance by undoing the min_heap transition.
>
> Link: https://lore.kernel.org/lkml/CAJhEC05+0S69z+3+FB2Cd0hD+pCRyWTKLEOsc8BOmH73p1m+KQ@mail.gmail.com
> Fixes: 866898efbb25 ("bcache: remove heap-related macros and switch to generic min_heap")
> Fixes: 92a8b224b833 ("lib/min_heap: introduce non-inline versions of min heap API functions")
> Reported-by: Robert Pang <robertpang@...gle.com>
> Closes: https://lore.kernel.org/linux-bcache/CAJhEC06F_AtrPgw2-7CvCqZgeStgCtitbD-ryuPpXQA-JG5XXw@mail.gmail.com
> Cc: stable@...r.kernel.org
> Signed-off-by: Kuan-Wei Chiu <visitorckw@...il.com>
Acked-by: Coly Li <colyli@...nel.org>
Thanks.
> ---
> drivers/md/bcache/alloc.c | 11 +++++++++--
> drivers/md/bcache/bset.c | 14 +++++++++++---
> drivers/md/bcache/extents.c | 10 +++++++++-
> drivers/md/bcache/movinggc.c | 10 +++++++++-
> 4 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
> index 8998e61efa40..da50f6661bae 100644
> --- a/drivers/md/bcache/alloc.c
> +++ b/drivers/md/bcache/alloc.c
> @@ -189,16 +189,23 @@ static inline bool new_bucket_min_cmp(const void *l, const void *r, void *args)
> return new_bucket_prio(ca, *lhs) < new_bucket_prio(ca, *rhs);
> }
>
> +static inline void new_bucket_swap(void *l, void *r, void __always_unused *args)
> +{
> + struct bucket **lhs = l, **rhs = r;
> +
> + swap(*lhs, *rhs);
> +}
> +
> static void invalidate_buckets_lru(struct cache *ca)
> {
> struct bucket *b;
> const struct min_heap_callbacks bucket_max_cmp_callback = {
> .less = new_bucket_max_cmp,
> - .swp = NULL,
> + .swp = new_bucket_swap,
> };
> const struct min_heap_callbacks bucket_min_cmp_callback = {
> .less = new_bucket_min_cmp,
> - .swp = NULL,
> + .swp = new_bucket_swap,
> };
>
> ca->heap.nr = 0;
> diff --git a/drivers/md/bcache/bset.c b/drivers/md/bcache/bset.c
> index 68258a16e125..bd97d8626887 100644
> --- a/drivers/md/bcache/bset.c
> +++ b/drivers/md/bcache/bset.c
> @@ -1093,6 +1093,14 @@ static inline bool new_btree_iter_cmp(const void *l, const void *r, void __alway
> return bkey_cmp(_l->k, _r->k) <= 0;
> }
>
> +static inline void new_btree_iter_swap(void *iter1, void *iter2, void __always_unused *args)
> +{
> + struct btree_iter_set *_iter1 = iter1;
> + struct btree_iter_set *_iter2 = iter2;
> +
> + swap(*_iter1, *_iter2);
> +}
> +
> static inline bool btree_iter_end(struct btree_iter *iter)
> {
> return !iter->heap.nr;
> @@ -1103,7 +1111,7 @@ void bch_btree_iter_push(struct btree_iter *iter, struct bkey *k,
> {
> const struct min_heap_callbacks callbacks = {
> .less = new_btree_iter_cmp,
> - .swp = NULL,
> + .swp = new_btree_iter_swap,
> };
>
> if (k != end)
> @@ -1149,7 +1157,7 @@ static inline struct bkey *__bch_btree_iter_next(struct btree_iter *iter,
> struct bkey *ret = NULL;
> const struct min_heap_callbacks callbacks = {
> .less = cmp,
> - .swp = NULL,
> + .swp = new_btree_iter_swap,
> };
>
> if (!btree_iter_end(iter)) {
> @@ -1223,7 +1231,7 @@ static void btree_mergesort(struct btree_keys *b, struct bset *out,
> : bch_ptr_invalid;
> const struct min_heap_callbacks callbacks = {
> .less = b->ops->sort_cmp,
> - .swp = NULL,
> + .swp = new_btree_iter_swap,
> };
>
> /* Heapify the iterator, using our comparison function */
> diff --git a/drivers/md/bcache/extents.c b/drivers/md/bcache/extents.c
> index 4b84fda1530a..a7221e5dbe81 100644
> --- a/drivers/md/bcache/extents.c
> +++ b/drivers/md/bcache/extents.c
> @@ -266,12 +266,20 @@ static bool new_bch_extent_sort_cmp(const void *l, const void *r, void __always_
> return !(c ? c > 0 : _l->k < _r->k);
> }
>
> +static inline void new_btree_iter_swap(void *iter1, void *iter2, void __always_unused *args)
> +{
> + struct btree_iter_set *_iter1 = iter1;
> + struct btree_iter_set *_iter2 = iter2;
> +
> + swap(*_iter1, *_iter2);
> +}
> +
> static struct bkey *bch_extent_sort_fixup(struct btree_iter *iter,
> struct bkey *tmp)
> {
> const struct min_heap_callbacks callbacks = {
> .less = new_bch_extent_sort_cmp,
> - .swp = NULL,
> + .swp = new_btree_iter_swap,
> };
> while (iter->heap.nr > 1) {
> struct btree_iter_set *top = iter->heap.data, *i = top + 1;
> diff --git a/drivers/md/bcache/movinggc.c b/drivers/md/bcache/movinggc.c
> index 45ca134cbf02..d6c73dd8eb2b 100644
> --- a/drivers/md/bcache/movinggc.c
> +++ b/drivers/md/bcache/movinggc.c
> @@ -190,6 +190,14 @@ static bool new_bucket_cmp(const void *l, const void *r, void __always_unused *a
> return GC_SECTORS_USED(*_l) >= GC_SECTORS_USED(*_r);
> }
>
> +static void new_bucket_swap(void *l, void *r, void __always_unused *args)
> +{
> + struct bucket **_l = l;
> + struct bucket **_r = r;
> +
> + swap(*_l, *_r);
> +}
> +
> static unsigned int bucket_heap_top(struct cache *ca)
> {
> struct bucket *b;
> @@ -204,7 +212,7 @@ void bch_moving_gc(struct cache_set *c)
> unsigned long sectors_to_move, reserve_sectors;
> const struct min_heap_callbacks callbacks = {
> .less = new_bucket_cmp,
> - .swp = NULL,
> + .swp = new_bucket_swap,
> };
>
> if (!c->copy_gc_enabled)
> --
> 2.34.1
>
--
Coly Li
Powered by blists - more mailing lists