From: Kuan-Wei Chiu <visitorckw@gmail.com>
To: akpm@linux-foundation.org, colyli@kernel.org,
kent.overstreet@linux.dev, robertpang@google.com
Cc: linux-kernel@vger.kernel.org, linux-bcache@vger.kernel.org,
jserv@ccns.ncku.edu.tw, Kuan-Wei Chiu <visitorckw@gmail.com>,
stable@vger.kernel.org
Subject: [PATCH v2 1/3] Revert "bcache: update min_heap_callbacks to use default builtin swap"
Date: Sun, 15 Jun 2025 04:23:51 +0800 [thread overview]
Message-ID: <20250614202353.1632957-2-visitorckw@gmail.com> (raw)
In-Reply-To: <20250614202353.1632957-1-visitorckw@gmail.com>
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@google.com>
Closes: https://lore.kernel.org/linux-bcache/CAJhEC06F_AtrPgw2-7CvCqZgeStgCtitbD-ryuPpXQA-JG5XXw@mail.gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
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
next prev parent reply other threads:[~2025-06-14 20:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-14 20:23 [PATCH v2 0/3] bcache: Revert min_heap migration due to performance regression Kuan-Wei Chiu
2025-06-14 20:23 ` Kuan-Wei Chiu [this message]
2025-06-15 5:51 ` [PATCH v2 1/3] Revert "bcache: update min_heap_callbacks to use default builtin swap" Coly Li
2025-06-14 20:23 ` [PATCH v2 2/3] Revert "bcache: remove heap-related macros and switch to generic min_heap" Kuan-Wei Chiu
2025-06-15 5:51 ` Coly Li
2025-06-14 20:23 ` [PATCH v2 3/3] bcache: Remove unnecessary select MIN_HEAP Kuan-Wei Chiu
2025-06-15 5:52 ` Coly Li
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=20250614202353.1632957-2-visitorckw@gmail.com \
--to=visitorckw@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=colyli@kernel.org \
--cc=jserv@ccns.ncku.edu.tw \
--cc=kent.overstreet@linux.dev \
--cc=linux-bcache@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robertpang@google.com \
--cc=stable@vger.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
all inboxes | Powered by JetHome®